Someoneā¢ should do it and produce
patches ;)
I will raise my hand once the discussion proceeded in such a direction š
Nitpick: readme still mentions them.
I did not yet adapt this with the last iteration as I wanted the code to
be somewhat "stable" first. It is updated now and should reflect the
current state.
Other standard header arguments that would be nice to support are
:prologue and :epilogue.
Added both.
`org-babel-execute:csharp' and `org-babel-execute:csharp' generate
NAMESPACE values independently.
This came as a consequence of removing some code with the capabilities
to produce "class libraries" with the last update. It is not a problem
for the execution of the code block. But yes, I am also not a fan of
that. Consequently, I removed it completely as it is not fundamentally
necessary with the current state.
Some other minor comments:
I resolved all of them (I think).
Next step will be writing tests.
The project now contains the file test-ob-csharp.el [1] along with a
test org-file ob-csharp-test.org [2]. I tried to follow the examples of
the test files under /testing/lisp/ and /testing/examples /of the
org-mode repository.
In addition, the FSF paperwork is complete and I got access to the emacs
repository.
[1] https://codeberg.org/buoso/csharp-babel/src/branch/dev/test-ob-csharp.el
[2]
https://codeberg.org/buoso/csharp-babel/src/branch/dev/ob-csharp-test.org
On 15.03.25 15:34, Ihor Radchenko wrote:
poverobuosodonati<poverobuosodon...@gmail.com> writes:
To me, it looks like this is going into the territory of compiled
sources being the result of evaluation, as we discussed in
https://list.orgmode.org/orgmode/1819406926.505980.1701990611...@fidget.co-bxl/
But that's a totally new behavior we may want to discuss across backends
before implemeting.
You are right: this went in exactly that direction. In fact, I was not
aware that there is this discussion in place and it is not intended to
(currently?) to let source blocks be consumers of compiled resources
from other source blocks.
In my mind, producing an executable file or project should be a special
:results type. But we need to discuss the exact logic in that thread
first and then actually implement it. Someoneā¢ should do it and produce
patches ;) I can participate in the discussion, but there is little
chance that I put this particular feature ahead of other priorities.
What I can tell is that I am on favor of adding such a feature to Org
babel.
... So, with this review iteration, the code
simplified quite a bit:
...
* "namespace" & "project" header arguments were removed (which also
was needed for the said purpose)
Nitpick: readme still mentions them.
Further, please note
* I forgot command line arguments, which was changed for this review
as well (source blocks accept a "cmdline" header argument now)
Other standard header arguments that would be nice to support are
:prologue and :epilogue.
https://codeberg.org/buoso/csharp-babel/src/commit/6f0c528bbdc0cd56c0639c630a11e80b25d1f74c/ob-csharp.el#L225-L230
The patch looks good in general now.
The only potential problem I noticed is that
`org-babel-execute:csharp' and `org-babel-execute:csharp' generate
NAMESPACE values independently. So, expanded body might be using
different namespace than project file.
Some other minor comments:
1. You need to add docstrings to all the functions
2. Add :package-version to all the defcustoms
3. Convert lambdas in defcustoms into proper functions. Otherwise, it
will be inconvenient to customize them.
4. I do not recommend using gensym. It can produce the same values in
different Emacs sessions (e.g. after restarting Emacs). Especially
for project-name that is used to create project folder - we risk
clashes. You may use make-temp-name to ensure unique file/directory
name.
5. You should probably remove messages in `org-babel-execute:csharp'.
At least, "executing ..." - it is duplicating message from
org-babel-execute-src-block.
Some users also dislike excessive messages, especially in batch Emacs
(seehttps://yhetil.org/emacs-devel/868qzd9hjg....@gnu.org/)
Next step will be writing tests.
