emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] ob-shell: consistent prefix
@ 2024-01-15 21:12 Matt
  2024-01-16 13:12 ` Ihor Radchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Matt @ 2024-01-15 21:12 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1924 bytes --]

The attached patch makes names in ob-shell.el use the same prefix
"org-babel-shell-" (along with a few similar minor consistency
updates).

Without the patch, three naming conventions exist:

- org-babel-shell
- org-babel-sh
- ob-shell <----- that's my fault :)

The Org Babel shell library was previously called "ob-sh.el" and later
changed to "ob-shell.el".  When this happened, the meaning of the "sh"
changed.  When the library was called "ob-sh.el", "sh" meant a generic
shell.  When the library was renamed "ob-shell.el", "shell" took on
the role of a generic shell and "sh" became a reference to /bin/sh.

The  current "sh" names are generic and not specific to "/bin/sh" or
something like the Bourne Shell.  The patch updates all "sh" names
to "shell".

When I submitted my async changes, I introduced an "ob-shell" prefix.
Emacs names typically begin with the library name and I just continued
in that style.  Org is different, however, because it's the "org-"
library.  For sub-libraries, like Babel, the convention is
"org-babel-<language>-".  ob-shell.el is unusual in that it supports
multiple languages.  However, ob-C.el also supports multiple languages
and uses "org-babel-<language>-".  I find there's no confusion using
this style.

FWIW, I found it confusing when I first started learning the Org code
base that the naming convention is at the "org-" level and not at the
library level (for example, "ob-shell-").  A large part of my
confusion, I think, was simply inconsistencies within ob-shell.

It's late for me, so rather than commit in a rush, I'm posting this
for review/comment.  The tests pass and I think I preserved all the
API function names (like org-babel-execute:template).

Thoughts?

--
Matt Trzcinski
Emacs Org contributor (ob-shell)
Learn more about Org mode at https://orgmode.org
Support Org development at https://liberapay.com/org-mode

[-- Attachment #2: 0001-make-name-prefixes-consistent.diff --]
[-- Type: application/octet-stream, Size: 11817 bytes --]

diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el
index 31135b5fb..21d44391a 100644
--- a/lisp/ob-shell.el
+++ b/lisp/ob-shell.el
@@ -80,7 +80,7 @@ is modified outside the Customize interface."
 	    (org-babel-execute:shell body params))))
       (put fname 'definition-name 'org-babel-shell-initialize))
     (defalias (intern (concat "org-babel-variable-assignments:" name))
-      #'org-babel-variable-assignments:shell
+      #'org-babel-shell-variable-assignments:shell
       (format "Return list of %s statements assigning to the block's \
 variables."
 	      name))
@@ -115,10 +115,10 @@ a shell execution being its exit code."
 (defun org-babel-execute:shell (body params)
   "Execute Shell BODY according to PARAMS.
 This function is called by `org-babel-execute-src-block'."
-  (let* ((session (org-babel-sh-initiate-session
+  (let* ((session (org-babel-shell-initiate-session
 		   (cdr (assq :session params))))
 	 (stdin (let ((stdin (cdr (assq :stdin params))))
-                  (when stdin (org-babel-sh-var-to-string
+                  (when stdin (org-babel-shell-var-to-string
                                (org-babel-ref-resolve stdin)))))
 	 (results-params (cdr (assq :result-params params)))
 	 (value-is-exit-status
@@ -129,10 +129,10 @@ This function is called by `org-babel-execute-src-block'."
 	 (cmdline (cdr (assq :cmdline params)))
          (full-body (concat
 		     (org-babel-expand-body:generic
-		      body params (org-babel-variable-assignments:shell params))
+		      body params (org-babel-shell-variable-assignments:shell params))
 		     (when value-is-exit-status "\necho $?"))))
     (org-babel-reassemble-table
-     (org-babel-sh-evaluate session full-body params stdin cmdline)
+     (org-babel-shell-evaluate session full-body params stdin cmdline)
      (org-babel-pick-name
       (cdr (assq :colname-names params)) (cdr (assq :colnames params)))
      (org-babel-pick-name
@@ -140,8 +140,8 @@ This function is called by `org-babel-execute-src-block'."
 
 (defun org-babel-prep-session:shell (session params)
   "Prepare SESSION according to the header arguments specified in PARAMS."
-  (let* ((session (org-babel-sh-initiate-session session))
-	 (var-lines (org-babel-variable-assignments:shell params)))
+  (let* ((session (org-babel-shell-initiate-session session))
+	 (var-lines (org-babel-shell-variable-assignments:shell params)))
     (org-babel-comint-in-buffer session
       (mapc (lambda (var)
               (insert var) (comint-send-input nil t)
@@ -160,27 +160,27 @@ This function is called by `org-babel-execute-src-block'."
 
 \f
 ;;; Helper functions
-(defun org-babel--variable-assignments:sh-generic
+(defun org-babel-shell--variable-assignments:default
     (varname values &optional sep hline)
   "Return a list of statements declaring the values as a generic variable."
-  (format "%s=%s" varname (org-babel-sh-var-to-sh values sep hline)))
+  (format "%s=%s" varname (org-babel-shell-var-to-shell values sep hline)))
 
-(defun org-babel--variable-assignments:fish
+(defun org-babel-shell--variable-assignments:fish
     (varname values &optional sep hline)
   "Return a list of statements declaring the values as a fish variable."
-  (format "set %s %s" varname (org-babel-sh-var-to-sh values sep hline)))
+  (format "set %s %s" varname (org-babel-shell-var-to-shell values sep hline)))
 
-(defun org-babel--variable-assignments:bash_array
+(defun org-babel-shell--variable-assignments:bash_array
     (varname values &optional sep hline)
   "Return a list of statements declaring the values as a bash array."
   (format "unset %s\ndeclare -a %s=( %s )"
 	  varname varname
 	  (mapconcat
-	   (lambda (value) (org-babel-sh-var-to-sh value sep hline))
+	   (lambda (value) (org-babel-shell-var-to-shell value sep hline))
 	   values
 	   " ")))
 
-(defun org-babel--variable-assignments:bash_assoc
+(defun org-babel-shell--variable-assignments:bash_assoc
     (varname values &optional sep hline)
   "Return a list of statements declaring the values as bash associative array."
   (format "unset %s\ndeclare -A %s\n%s"
@@ -189,22 +189,22 @@ This function is called by `org-babel-execute-src-block'."
 	   (lambda (items)
 	     (format "%s[%s]=%s"
 		     varname
-		     (org-babel-sh-var-to-sh (car items) sep hline)
-		     (org-babel-sh-var-to-sh (cdr items) sep hline)))
+		     (org-babel-shell-var-to-shell (car items) sep hline)
+		     (org-babel-shell-var-to-shell (cdr items) sep hline)))
 	   values
 	   "\n")))
 
-(defun org-babel--variable-assignments:bash (varname values &optional sep hline)
+(defun org-babel-shell--variable-assignments:bash (varname values &optional sep hline)
   "Represent the parameters as useful Bash shell variables."
   (pcase values
     (`((,_ ,_ . ,_) . ,_)		;two-dimensional array
-     (org-babel--variable-assignments:bash_assoc varname values sep hline))
+     (org-babel-shell--variable-assignments:bash_assoc varname values sep hline))
     (`(,_ . ,_)				;simple list
-     (org-babel--variable-assignments:bash_array varname values sep hline))
+     (org-babel-shell--variable-assignments:bash_array varname values sep hline))
     (_					;scalar value
-     (org-babel--variable-assignments:sh-generic varname values sep hline))))
+     (org-babel-shell--variable-assignments:default varname values sep hline))))
 
-(defun org-babel-variable-assignments:shell (params)
+(defun org-babel-shell-variable-assignments:shell (params)
   "Return list of shell statements assigning the block's variables."
   (let ((sep (cdr (assq :separator params)))
 	(hline (when (string= "yes" (cdr (assq :hlines params)))
@@ -213,25 +213,25 @@ This function is called by `org-babel-execute-src-block'."
     (mapcar
      (lambda (pair)
        (if (string-suffix-p "bash" shell-file-name)
-	   (org-babel--variable-assignments:bash
+	   (org-babel-shell--variable-assignments:bash
             (car pair) (cdr pair) sep hline)
          (if (string-suffix-p "fish" shell-file-name)
-	     (org-babel--variable-assignments:fish
+	     (org-babel-shell--variable-assignments:fish
               (car pair) (cdr pair) sep hline)
-           (org-babel--variable-assignments:sh-generic
+           (org-babel-shell--variable-assignments:default
 	    (car pair) (cdr pair) sep hline))))
      (org-babel--get-vars params))))
 
-(defun org-babel-sh-var-to-sh (var &optional sep hline)
+(defun org-babel-shell-var-to-shell (var &optional sep hline)
   "Convert an elisp value to a shell variable.
 Convert an elisp var into a string of shell commands specifying a
 var of the same value."
   (concat "'" (replace-regexp-in-string
 	       "'" "'\"'\"'"
-	       (org-babel-sh-var-to-string var sep hline))
+	       (org-babel-shell-var-to-string var sep hline))
 	  "'"))
 
-(defun org-babel-sh-var-to-string (var &optional sep hline)
+(defun org-babel-shell-var-to-string (var &optional sep hline)
   "Convert an elisp value to a string."
   (let ((echo-var (lambda (v) (if (stringp v) v (format "%S" v)))))
     (cond
@@ -242,14 +242,14 @@ var of the same value."
       (mapconcat echo-var var "\n"))
      (t (funcall echo-var var)))))
 
-(defvar org-babel-sh-eoe-indicator "echo 'org_babel_sh_eoe'"
+(defvar org-babel-shell-eoe-indicator "echo 'org_babel_sh_eoe'"
   "String to indicate that evaluation has completed.")
-(defvar org-babel-sh-eoe-output "org_babel_sh_eoe"
+(defvar org-babel-shell-eoe-output "org_babel_sh_eoe"
   "String to indicate that evaluation has completed.")
-(defvar org-babel-sh-prompt "org_babel_sh_prompt> "
+(defvar org-babel-shell-prompt "org_babel_sh_prompt> "
   "String to set prompt in session shell.")
 
-(defun org-babel-sh-initiate-session (&optional session _params)
+(defun org-babel-shell-initiate-session (&optional session _params)
   "Initiate a session named SESSION according to PARAMS."
   (when (and session (not (string= session "none")))
     (save-window-excursion
@@ -264,9 +264,9 @@ var of the same value."
               (or (cdr (assoc (file-name-nondirectory shell-file-name)
                               org-babel-shell-set-prompt-commands))
                   (alist-get t org-babel-shell-set-prompt-commands))
-              org-babel-sh-prompt))
+              org-babel-shell-prompt))
             (setq-local comint-prompt-regexp
-                        (concat "^" (regexp-quote org-babel-sh-prompt)
+                        (concat "^" (regexp-quote org-babel-shell-prompt)
                                 " *"))
 	    ;; Needed for Emacs 23 since the marker is initially
 	    ;; undefined and the filter functions try to use it without
@@ -274,16 +274,16 @@ var of the same value."
 	    (set-marker comint-last-output-start (point))
 	    (get-buffer (current-buffer)))))))
 
-(defconst ob-shell-async-indicator "echo 'ob_comint_async_shell_%s_%s'"
+(defconst org-babel-shell-async-indicator "echo 'ob_comint_async_shell_%s_%s'"
   "Session output delimiter template.
 See `org-babel-comint-async-indicator'.")
 
-(defun ob-shell-async-chunk-callback (string)
+(defun org-babel-shell-async-chunk-callback (string)
   "Filter applied to results before insertion.
 See `org-babel-comint-async-chunk-callback'."
   (replace-regexp-in-string comint-prompt-regexp "" string))
 
-(defun org-babel-sh-evaluate (session body &optional params stdin cmdline)
+(defun org-babel-shell-evaluate (session body &optional params stdin cmdline)
   "Pass BODY to the Shell process in BUFFER.
 If RESULT-TYPE equals `output' then return a list of the outputs
 of the statements in BODY, if RESULT-TYPE equals `value' then
@@ -328,28 +328,28 @@ return the value of the last statement in BODY."
                      session
                      (current-buffer)
                      "ob_comint_async_shell_\\(.+\\)_\\(.+\\)"
-                     'ob-shell-async-chunk-callback
+                     'org-babel-shell-async-chunk-callback
                      nil)
                     (org-babel-comint-async-delete-dangling-and-eval
                         session
-                      (insert (format ob-shell-async-indicator "start" uuid))
+                      (insert (format org-babel-shell-async-indicator "start" uuid))
                       (comint-send-input nil t)
                       (insert (org-trim body))
                       (comint-send-input nil t)
-                      (insert (format ob-shell-async-indicator "end" uuid))
+                      (insert (format org-babel-shell-async-indicator "end" uuid))
                       (comint-send-input nil t))
                     uuid))
 	      (mapconcat
-	       #'org-babel-sh-strip-weird-long-prompt
+	       #'org-babel-shell-strip-weird-long-prompt
 	       (mapcar
 	        #'org-trim
 	        (butlast ; Remove eoe indicator
 	         (org-babel-comint-with-output
-		     (session org-babel-sh-eoe-output t body)
+		     (session org-babel-shell-eoe-output t body)
                    (insert (org-trim body) "\n"
-                           org-babel-sh-eoe-indicator)
+                           org-babel-shell-eoe-indicator)
 		   (comint-send-input nil t))
-                 ;; Remove `org-babel-sh-eoe-indicator' output line.
+                 ;; Remove `org-babel-shell-eoe-indicator' output line.
 	         1))
 	       "\n")))
 	   ;; External shell script, with or without a predefined
@@ -380,7 +380,7 @@ return the value of the last statement in BODY."
             (with-temp-file tmp-file (insert results))
             (org-babel-import-elisp-from-file tmp-file)))))))
 
-(defun org-babel-sh-strip-weird-long-prompt (string)
+(defun org-babel-shell-strip-weird-long-prompt (string)
   "Remove prompt cruft from a string of shell output."
   (while (string-match "^% +[\r\n$]+ *" string)
     (setq string (substring string (match-end 0))))

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-01-20 16:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-15 21:12 [PATCH] ob-shell: consistent prefix Matt
2024-01-16 13:12 ` Ihor Radchenko
2024-01-20 16:03   ` Matt

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).