Matt <m...@excalamus.com> writes: > > > +;; TODO refactor into macro. Currently violates (elisp) Coding > > > +;; Conventions and is hard to debug. > > > (defun org-babel-shell-initialize () > > > "Define execution functions associated to shell names. > > > > Could you please elaborate? Which particular convention does it violate? > > What is hard to debug? > > (elisp) Coding Conventions says, > > "• Constructs that define a function or variable should be macros, not > functions, and their names should start with ‘define-’. The macro > should receive the name to be defined as the first argument. That > will help various tools find the definition automatically. Avoid > constructing the names in the macro itself, since that would > confuse these tools." > > The `org-babel-shell-initialize' function defines *all* the > `org-babel-execute:XXX' functions given by `org-babel-shell-names' (sh, bash, > zsh, etc.).
I agree that `org-babel-shell-initialize' could use a better name. As for being a macro, there will be not much gain - the convention is mostly designed for things like `cl-defun' aimed to be used in the code. `org-babel-shell-initialize' is only used by `org-babel-shell-names'. I do not have objections if it were a macro though. (But I do not see how it would help debugging). > Because `org-babel-shell-initialize' is a function factory, you can't easily > examine or modify their definitions. `C-h f org-babel-execute:sh' jumps to > the top of lisp/ob-shell.el. Changing the definition requires reevaluating > the definition for all the execute functions (or first changing > `org-babel-shell-names'). This is indeed a downside. Any better ideas? ob-core dictates that we must have org-babel-execute:lang functions to make things work. > This was a problem for me when I wanted to make the session name string for > `test-ob-shell/session' the test name (mentioned above). In the test, when I > replaced the session name string with a variable containing the string, > `org-babel-execute:sh' failed with a type error. I couldn't get the variable > to evaluate (with backquote and comma or otherwise). Without an explicit > function definition or a macro to expand, I found it hard to debug/experiment > with (and so left the test name as a hard coded string). Could you please explain a bit more about the problem? I do not see how macro would help in this situation. > I probably don't need it and am happy to remove it. An older version of > the function was more complex and made sense as a separate function (or so I > thought). My aim was to make the test strings easy to read so that it's > clearer what's being tested (i.e. not write multi-line strings on a single > line). I could use concat and add "\n" to the end of each line. Or, simply > write out the string-join. Maybe there's another way to write multi-line > strings that I'm not aware of in Elisp, maybe something like Python's > triple-quote? We write multi-line docstrings all the time without extra macros. I recommend using paredit or similar packages to auto-escape things that need to be escaped. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92>
