emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] ob-shell-test, test-ob-shell and introduction
@ 2021-11-22 18:37 Matt
  2021-11-22 18:43 ` Timothy
  2021-11-22 19:08 ` Thomas S. Dye
  0 siblings, 2 replies; 19+ messages in thread
From: Matt @ 2021-11-22 18:37 UTC (permalink / raw)
  To: emacs-orgmode

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

Hi,

I'm interested in getting async into ob-shell.el. Since I've never contributed before, I figure it'd be good to start with a few easy tasks.

It looks to me like the stdin and cmdline header args aren't documented anywhere (at least I couldn't find anything). To make sure I'm using them correctly before making a patch for the manual, here are some tests.

Please let me know if things look okay. It wasn't clear to me how to send along a message with git send-email, so I formatted these patches and included them as an attachment. Is that fine?

Thanks,
Matt

[-- Attachment #2: 0001-ob-shell-test.org-Add-example-for-stdin.patch --]
[-- Type: application/octet-stream, Size: 960 bytes --]

From b59ec4c2d949ca4c2d881238393a8de3851315db Mon Sep 17 00:00:00 2001
From: Matt <matt@excalamus.com>
Date: Mon, 22 Nov 2021 13:07:53 -0500
Subject: [PATCH 1/2] ob-shell-test.org:  Add example for stdin

* ob-shell-test.org Add section for testing header args.
---
 testing/examples/ob-shell-test.org | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/testing/examples/ob-shell-test.org b/testing/examples/ob-shell-test.org
index 2510f4f96..39539dc9f 100644
--- a/testing/examples/ob-shell-test.org
+++ b/testing/examples/ob-shell-test.org
@@ -86,3 +86,22 @@ echo ${table[spaghetti]}
 #+RESULTS:
 : 20 cm
 
+* Header arg tests
+  :PROPERTIES:
+  :ID:       cc56f3c6-13ec-4026-9d83-3106efd833e2
+  :END:
+
+** stdin
+#+name: an-org-reference
+org reference line 1
+org reference line 2
+
+#+begin_src sh :stdin an-org-reference :results output
+cat
+#+end_src
+
+#+RESULTS:
+: org reference line 1
+: org reference line 2
+
+
-- 
2.34.0


[-- Attachment #3: 0002-test-ob-shell.el-Add-tests-for-stdin-and-cmdline-hea.patch --]
[-- Type: application/octet-stream, Size: 1364 bytes --]

From b923d17679ba9c7cb88dd40312534c33eff74dd8 Mon Sep 17 00:00:00 2001
From: Matt <matt@excalamus.com>
Date: Mon, 22 Nov 2021 13:14:01 -0500
Subject: [PATCH 2/2] test-ob-shell.el: Add tests for stdin and cmdline header
 args

* test-ob-shell.el (test-ob-shell/stdin, test-ob-shell/cmdline) Add
tests.
---
 testing/lisp/test-ob-shell.el | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/testing/lisp/test-ob-shell.el b/testing/lisp/test-ob-shell.el
index 2f346f699..b9fcb3819 100644
--- a/testing/lisp/test-ob-shell.el
+++ b/testing/lisp/test-ob-shell.el
@@ -106,6 +106,19 @@ ob-comint.el, which was not previously tested."
 	   "#+BEGIN_SRC sh :results output :var l='(1 2)\necho ${l}\n#+END_SRC"
 	   (org-trim (org-babel-execute-src-block))))))
 
+(ert-deftest test-ob-shell/stdin ()
+  "Confirm stdin passes an org reference"
+  (org-test-at-id "cc56f3c6-13ec-4026-9d83-3106efd833e2"
+    (org-babel-next-src-block)
+    (should (equal "org reference line 1\norg reference line 2"
+                   (org-trim (org-babel-execute-src-block))))))
+
+(ert-deftest test-ob-shell/cmdline ()
+  "Confirm cmdline header gets passed in as an argument."
+  (let ((res (org-babel-execute:sh "echo $1;" '((:cmdline . "foo")))))
+    (should res)
+    (should (equal "foo" res))))
+
 (provide 'test-ob-shell)
 
 ;;; test-ob-shell.el ends here
-- 
2.34.0


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-11-22 18:37 [PATCH] ob-shell-test, test-ob-shell and introduction Matt
@ 2021-11-22 18:43 ` Timothy
  2021-11-24 18:48   ` Matt
  2021-11-22 19:08 ` Thomas S. Dye
  1 sibling, 1 reply; 19+ messages in thread
From: Timothy @ 2021-11-22 18:43 UTC (permalink / raw)
  To: Matt; +Cc: emacs-orgmode

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

Hi Matt,

> I’m interested in getting async into ob-shell.el. Since I’ve never contributed
>before, I figure it’d be good to start with a few easy tasks.

Fantastic! Great to hear from you, and I hope it goes well. Feel free to send
further emails for if you get stuck. If you intend to do more than just one or
two small tweaks (it sounds like you have bigger plans than that), you’ll need
what’s known as [FSF copyright assignment]. Have you done that yet?

> It looks to me like the stdin and cmdline header args aren’t documented anywhere
> (at least I couldn’t find anything). To make sure I’m using them correctly
> before making a patch for the manual, here are some tests.

Great! I’m not overly familiar with these, so I’ll leave other people to take a
look at the tests you’ve written, but your approach sounds good. 👍

> Please let me know if things look okay. It wasn’t clear to me how to send along
> a message with git send-email, so I formatted these patches and included them as
> an attachment. Is that fine?

Patches attached to an email are perfectly fine here.
Thanks for getting in touch, and I hope to see you around 🙂.

All the best,
Timothy


[FSF copyright assignment] <https://orgmode.org/contribute.html#copyright>

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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-11-22 18:37 [PATCH] ob-shell-test, test-ob-shell and introduction Matt
  2021-11-22 18:43 ` Timothy
@ 2021-11-22 19:08 ` Thomas S. Dye
  2021-11-23  4:42   ` [PATCH] ob-doc-shell.org (was [PATCH] ob-shell-test, test-ob-shell and introduction) Matt
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas S. Dye @ 2021-11-22 19:08 UTC (permalink / raw)
  To: emacs-orgmode

Aloha Matt,

Matt <matt@excalamus.com> writes:

> Hi,
>
> I'm interested in getting async into ob-shell.el. Since I've 
> never contributed before, I figure it'd be good to start with a 
> few easy tasks.
>
> It looks to me like the stdin and cmdline header args aren't 
> documented anywhere (at least I couldn't find anything). To make 
> sure I'm using them correctly before making a patch for the 
> manual, here are some tests.
>
> Please let me know if things look okay. It wasn't clear to me 
> how to send along a message with git send-email, so I formatted 
> these patches and included them as an attachment. Is that fine?
>
> Thanks,
> Matt

Unfortunately, there is no ob-doc-shell.org at 
worg/org-contrib/babel/languages/.

There is an ob-doc-template.org in case you'd like to contribute 
ob-doc-shell.org :)

All the best,
Tom
-- 
Thomas S. Dye
https://tsdye.online/tsdye


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

* [PATCH] ob-doc-shell.org (was [PATCH] ob-shell-test, test-ob-shell and introduction)
  2021-11-22 19:08 ` Thomas S. Dye
@ 2021-11-23  4:42   ` Matt
  2021-11-23  4:59     ` Thomas S. Dye
  0 siblings, 1 reply; 19+ messages in thread
From: Matt @ 2021-11-23  4:42 UTC (permalink / raw)
  To: Thomas S. Dye; +Cc: emacs-orgmode

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

 > Unfortunately, there is no ob-doc-shell.org at 
 > worg/org-contrib/babel/languages/.
 > 
 > There is an ob-doc-template.org in case you'd like to contribute 
 > ob-doc-shell.org :)

I created one, along with a sourcehut account. However, it looks like I need to be granted some permissions to write to the worg website. Is Bastien the contact for that? I've attached a patch in the mean time.

[-- Attachment #2: 0001-org-contrib-babel-languages-ob-doc-shell.org-Create-.patch --]
[-- Type: application/octet-stream, Size: 4295 bytes --]

From 682eb6bd14d7d7246e3f13517adcd9ffb1a38ae0 Mon Sep 17 00:00:00 2001
From: Matt <matt@excalamus.com>
Date: Mon, 22 Nov 2021 22:57:38 -0500
Subject: [PATCH] org-contrib/babel/languages/ob-doc-shell.org: Create file

* ob-doc-shell.org Create file.  Fill out links and images.
---
 org-contrib/babel/languages/ob-doc-shell.org | 86 ++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 org-contrib/babel/languages/ob-doc-shell.org

diff --git a/org-contrib/babel/languages/ob-doc-shell.org b/org-contrib/babel/languages/ob-doc-shell.org
new file mode 100644
index 00000000..46ce0a6f
--- /dev/null
+++ b/org-contrib/babel/languages/ob-doc-shell.org
@@ -0,0 +1,86 @@
+#+OPTIONS:    H:3 num:nil toc:2 \n:nil ::t |:t ^:{} -:t f:t *:t tex:t d:(HIDE) tags:not-in-toc broken-links:nil
+#+STARTUP:    align fold nodlcheck hidestars oddeven lognotestate hideblocks
+#+SEQ_TODO:   TODO(t) INPROGRESS(i) WAITING(w@) | DONE(d) CANCELED(c@)
+#+TAGS:       Write(w) Update(u) Fix(f) Check(c) noexport(n)
+#+TITLE:      Shell Source Code Blocks in Org Mode
+#+AUTHOR:     Matt Trzcinski
+#+EMAIL:      matt[at]excalamus[dot]com
+#+LANGUAGE:   en
+#+HTML_LINK_UP:    index.html
+#+HTML_LINK_HOME:  https://orgmode.org/worg/
+#+EXCLUDE_TAGS: noexport
+
+#+name: banner
+#+begin_export html
+<div id="subtitle" style="float: center; text-align: center;">
+  <p>
+    Babel shell support, including: <a href="https://www.gnu.org/software/bash/">sh</a>,
+    <a href="https://www.gnu.org/software/bash/">bash</a>,
+    <a href="https://www.zsh.org/">zsh</a>,
+    <a href="https://fishshell.com/">fish</a>,
+    <a href="https://www.grymoire.com/unix/csh.html">csh</a>,
+    <a href="https://www.in-ulm.de/~mascheck/various/ash/">ash</a>,
+    <a href="http://gondor.apana.org.au/~herbert/dash/">dash</a>,
+    <a href="http://www.kornshell.org/">ksh</a>,
+    <a href="https://www.mirbsd.org/mksh.htm">mksh</a>, and
+    <a href="https://packages.qa.debian.org/p/posh.html">posh</a>.
+  </p>
+  <p>
+    <a href="https://www.gnu.org/software/bash/">
+      <img src="https://tiswww.case.edu/php/chet/img/bash-logo-web.png"/>
+    </a>
+    <a href="https://www.zsh.org/">zsh</a>,
+      <img src="https://zsh.sourceforge.io/Images/wizard.gif"/>
+    </a>
+    <a href="https://fishshell.com/">fish</a>,
+      <img src="https://fishshell.com/assets/img/Terminal_Logo2_CRT_Flat.png"/>
+    </a>
+    <a href="http://www.kornshell.org/">ksh</a>,
+      <img src="http://www.kornshell.org/kornshell.gif"/>
+    </a>
+    <a href="https://www.mirbsd.org/mksh.htm">mksh</a>, and
+      <img src="https://www.mirbsd.org/pics/logo-grey.png"/>
+    </a>
+  </p>
+</div>
+#+end_export
+
+* Template Checklist [4/12]                                        :noexport:
+  - [X] Revise #+TITLE:
+  - [X] Indicate #+AUTHOR:
+  - [X] Add #+EMAIL:
+  - [X] Revise banner source block [3/3]
+    - [X] Add link to a useful language web site
+    - [X] Replace "Language" with language name
+    - [X] Find a suitable graphic and use it to link to the language
+      web site
+  - [ ] Write an [[Introduction]]
+  - [ ] Describe [[Requirements and Setup][Requirements and Setup]]
+  - [ ] Replace "Language" with language name in [[Org Mode Features for Language Source Code Blocks][Babel Features for Language Source Code Blocks]]
+  - [ ] Describe [[Header Arguments][Header Arguments]]
+  - [ ] Describe support for [[Sessions]]
+  - [ ] Describe [[Result Types][Result Types]]
+  - [ ] Describe [[Other]] differences from supported languages
+  - [ ] Provide brief [[Examples of Use][Examples of Use]]
+* Introduction
+  - Brief description of language.
+  - Range of typical uses within Org Mode.
+* Requirements and Setup
+  - Installation and configuration of language software
+  - Emacs configuration
+  - Org-mode configuration (org-babel-do-load-languages)
+* Babel Features for Language Source Code Blocks
+** Header Arguments
+   - Language-specific default values
+   - Language-specific header arguments
+** Sessions
+   - Support or not
+   - Typical use for sessions
+** Result Types
+   - Which result types are supported?
+** Other
+   - Differences from other supported languages
+* Examples of Use
+  - Hello World!
+  - Common uses
+  - Links to tutorials and other resources
-- 
2.34.0


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

* Re: [PATCH] ob-doc-shell.org (was [PATCH] ob-shell-test, test-ob-shell and introduction)
  2021-11-23  4:42   ` [PATCH] ob-doc-shell.org (was [PATCH] ob-shell-test, test-ob-shell and introduction) Matt
@ 2021-11-23  4:59     ` Thomas S. Dye
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas S. Dye @ 2021-11-23  4:59 UTC (permalink / raw)
  To: Matt; +Cc: emacs-orgmode

Aloha Matt,
Matt <matt@excalamus.com> writes:

>  > Unfortunately, there is no ob-doc-shell.org at 
>  > worg/org-contrib/babel/languages/.
>  > 
>  > There is an ob-doc-template.org in case you'd like to 
>  > contribute 
>  > ob-doc-shell.org :)
>
> I created one, along with a sourcehut account. However, it looks 
> like I need to be granted some permissions to write to the worg 
> website. Is Bastien the contact for that? I've attached a patch 
> in the mean time.

That's a good start.  Thanks!

Yes, contact Bastien directly off list with your sourcehut user 
name and he'll set you up to push directly to Worg.

Let me know if you have questions.

All the best,
Tom

-- 
Thomas S. Dye
https://tsdye.online/tsdye


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-11-22 18:43 ` Timothy
@ 2021-11-24 18:48   ` Matt
  2021-12-02  9:39     ` Timothy
  0 siblings, 1 reply; 19+ messages in thread
From: Matt @ 2021-11-24 18:48 UTC (permalink / raw)
  To: Timothy; +Cc: emacs-orgmode


 > [FSF copyright assignment]. Have you done that yet?
 
I just verified with my employer that my contract grants an exception for this project.  Just emailed the request to assign@gnu.org.  Also, got access from Bastien for worg. I figure it's probably best to reserve any more changes 'til the paper work is done?
 



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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-11-24 18:48   ` Matt
@ 2021-12-02  9:39     ` Timothy
  2021-12-06  3:50       ` Matt
  0 siblings, 1 reply; 19+ messages in thread
From: Timothy @ 2021-12-02  9:39 UTC (permalink / raw)
  To: Matt; +Cc: emacs-orgmode

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

Hi Matt,

>  > [FSF copyright assignment]. Have you done that yet?
>
> I just verified with my employer that my contract grants an exception for this
> project. Just emailed the request to assign@gnu.org. Also, got access from
> Bastien for worg. I figure it’s probably best to reserve any more changes ’til
> the paper work is done?

Great to hear that won’t get in the way of things 🙂. Feel free to develop and
share patches before the assignment is complete, we’ll just wait till it’s gone
through before merging (the process should just be a quick email or two and take
no more than a few days, just let us know if it’s dragging out).

Hope that helps.

All the best,
Timothy

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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-02  9:39     ` Timothy
@ 2021-12-06  3:50       ` Matt
  2021-12-06  4:50         ` Thomas S. Dye
  0 siblings, 1 reply; 19+ messages in thread
From: Matt @ 2021-12-06  3:50 UTC (permalink / raw)
  To: Timothy; +Cc: emacs-orgmode

 > > I just verified with my employer that my contract grants an exception for this
 > > project. Just emailed the request to assign@gnu.org. 

Not surprisingly the FSF hasn't resources to verify my contract's exception and needs a written disclaimer from my employer. I'm waiting on this now.

 > Feel free to develop and
 > share patches before the assignment is complete, we’ll just wait till it’s gone
 > through before merging 
 
Given the contract exception, I'll start moving forward again with the assumption that I'll eventually get that written disclaimer.

 > Hope that helps.
 
It does, thank you. It's nice to know that my words aren't going into the void. :)


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-06  3:50       ` Matt
@ 2021-12-06  4:50         ` Thomas S. Dye
  2021-12-18  7:03           ` Matt
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas S. Dye @ 2021-12-06  4:50 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Timothy

Aloha Matt,

Matt <matt@excalamus.com> writes:

>  > > I just verified with my employer that my contract grants an 
>  > > exception for this
>  > > project. Just emailed the request to assign@gnu.org. 
>
> Not surprisingly the FSF hasn't resources to verify my 
> contract's exception and needs a written disclaimer from my 
> employer. I'm waiting on this now.
>
>  > Feel free to develop and
>  > share patches before the assignment is complete, we’ll just 
>  > wait till it’s gone
>  > through before merging 
>  
> Given the contract exception, I'll start moving forward again 
> with the assumption that I'll eventually get that written 
> disclaimer.
>
>  > Hope that helps.
>  
> It does, thank you. It's nice to know that my words aren't going 
> into the void. :)

Good news.

Contributions to Worg aren't similarly restricted.  Feel free to 
push material there in the meantime.

All the best,
Tom

-- 
Thomas S. Dye
https://tsdye.online/tsdye



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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-06  4:50         ` Thomas S. Dye
@ 2021-12-18  7:03           ` Matt
  2021-12-18 20:51             ` Thomas S. Dye
  2021-12-31 17:04             ` Thomas S. Dye
  0 siblings, 2 replies; 19+ messages in thread
From: Matt @ 2021-12-18  7:03 UTC (permalink / raw)
  To: Thomas S. Dye; +Cc: emacs-orgmode, Timothy

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

 > Contributions to Worg aren't similarly restricted.  Feel free to 
 > push material there in the meantime.

Looks like the email finally got sent to the right person at my company. Who knows how long it will take for them to get the FSF disclaimer back to me...

Now that I'm on holiday, I've got a little more time to contribute to Worg. Here's a few patches showing what I've come up with. I'll finish it up in the next few days, hopefully.  Not sure if the patches are formed correctly,  sorry about that if not. It's late here and I wanted to show progress. I'll worry about those kinds of details later. :)

[-- Attachment #2: 0001-Draft-introduction-and-setup.patch --]
[-- Type: application/octet-stream, Size: 6862 bytes --]

From 0d12f927d049749f960a5b4385c1f8029e859a3b Mon Sep 17 00:00:00 2001
From: Matt <matt@excalamus.com>
Date: Sat, 18 Dec 2021 01:46:29 -0500
Subject: [PATCH] Draft introduction and setup

---
 org-contrib/babel/languages/ob-doc-shell.org | 141 ++++++++++++++++---
 1 file changed, 120 insertions(+), 21 deletions(-)

diff --git a/org-contrib/babel/languages/ob-doc-shell.org b/org-contrib/babel/languages/ob-doc-shell.org
index 46ce0a6f..825e17d2 100644
--- a/org-contrib/babel/languages/ob-doc-shell.org
+++ b/org-contrib/babel/languages/ob-doc-shell.org
@@ -2,7 +2,7 @@
 #+STARTUP:    align fold nodlcheck hidestars oddeven lognotestate hideblocks
 #+SEQ_TODO:   TODO(t) INPROGRESS(i) WAITING(w@) | DONE(d) CANCELED(c@)
 #+TAGS:       Write(w) Update(u) Fix(f) Check(c) noexport(n)
-#+TITLE:      Shell Source Code Blocks in Org Mode
+#+TITLE:      Shell Code Blocks in Babel
 #+AUTHOR:     Matt Trzcinski
 #+EMAIL:      matt[at]excalamus[dot]com
 #+LANGUAGE:   en
@@ -10,6 +10,9 @@
 #+HTML_LINK_HOME:  https://orgmode.org/worg/
 #+EXCLUDE_TAGS: noexport
 
+# TODO remove this
+#+options: toc:nil
+
 #+name: banner
 #+begin_export html
 <div id="subtitle" style="float: center; text-align: center;">
@@ -26,21 +29,21 @@
     <a href="https://packages.qa.debian.org/p/posh.html">posh</a>.
   </p>
   <p>
-    <a href="https://www.gnu.org/software/bash/">
-      <img src="https://tiswww.case.edu/php/chet/img/bash-logo-web.png"/>
-    </a>
-    <a href="https://www.zsh.org/">zsh</a>,
-      <img src="https://zsh.sourceforge.io/Images/wizard.gif"/>
-    </a>
-    <a href="https://fishshell.com/">fish</a>,
-      <img src="https://fishshell.com/assets/img/Terminal_Logo2_CRT_Flat.png"/>
-    </a>
-    <a href="http://www.kornshell.org/">ksh</a>,
-      <img src="http://www.kornshell.org/kornshell.gif"/>
-    </a>
-    <a href="https://www.mirbsd.org/mksh.htm">mksh</a>, and
-      <img src="https://www.mirbsd.org/pics/logo-grey.png"/>
-    </a>
+    <a href="https://www.gnu.org/software/bash/"><img width="180px"
+    src="https://tiswww.case.edu/php/chet/img/bash-logo-web.png"/></a>
+    
+    <a href="https://www.zsh.org/"><img width="150px"
+    src="https://zsh.sourceforge.io/Images/wizard.gif"/></a>
+
+    <a href="https://www.mirbsd.org/mksh.htm"><img width="150px"
+    src="https://www.mirbsd.org/pics/logo-grey.png"/></a>
+    
+    <a href="https://fishshell.com/"><img width="150px"
+    src="https://fishshell.com/assets/img/Terminal_Logo2_CRT_Flat.png"/></a>
+    
+    <a href="http://www.kornshell.org/"><img width="180px"
+    src="http://www.kornshell.org/kornshell.gif"/></a>
+
   </p>
 </div>
 #+end_export
@@ -63,12 +66,84 @@
   - [ ] Describe [[Other]] differences from supported languages
   - [ ] Provide brief [[Examples of Use][Examples of Use]]
 * Introduction
-  - Brief description of language.
-  - Range of typical uses within Org Mode.
+# - Brief description of language.
+A shell is a user interface for interacting with system services.
+File management, process execution, and operating system monitoring
+can all be done with a shell. Many shells are plain text only whereas
+some support graphics or are themselves fully graphical. Similarities
+exist between shells and, though standards exist (such as
+POSIX[fn:1]), there is no guarantee that what works in one shell will
+work in another. Each shell is a separate application.
+
+Shells often provide a programming language as well as access to
+system utilities, such as the GNU Core Utilities[fn:2]. Users can
+stitch utilities together or create their own.
+
+# - Range of typical uses within Org Mode.
+Org Babel lets users run commands[fn:3] in separate shells...
+
+: #+begin_src sh :results output
+:   echo PID: $$
+: #+end_src
+: 
+: #+RESULTS:
+: : PID: 9952
+: 
+: #+begin_src sh :results output 
+:   echo PID: $$
+: #+end_src
+: 
+: #+RESULTS:
+: : PID: 9990
+: 
+...or as part of a shared session.
+
+: #+begin_src sh :results output :session shared
+:   echo PID: $$
+:   export X=1
+: #+end_src
+: 
+: #+RESULTS:
+: : PID: 9742
+: 
+: #+begin_src sh :results output :session shared
+:   echo PID: $$
+:   echo X was set to $X
+: #+end_src
+: 
+: #+RESULTS:
+: : PID: 9742
+: : X was set to 1
+
+Shell blocks can run as standalone scripts or chained processes, be
+tangled, use args, work with stdin, have shebangs, and more.
+    
 * Requirements and Setup
-  - Installation and configuration of language software
-  - Emacs configuration
-  - Org-mode configuration (org-babel-do-load-languages)
+# - Installation and configuration of language software
+Org Babel can run many different shells, including [[https://www.gnu.org/software/bash/][sh]], [[https://www.gnu.org/software/bash/][bash]], [[https://www.zsh.org/][zsh]],
+[[https://fishshell.com/][fish]], [[https://www.grymoire.com/unix/csh.html][csh]], [[https://www.in-ulm.de/~mascheck/various/ash/][ash]], [[http://gondor.apana.org.au/~herbert/dash/][dash]], [[http://www.kornshell.org/][ksh]], [[https://www.mirbsd.org/mksh.htm][mksh]], and [[https://packages.qa.debian.org/p/posh.html][posh]].  The primary requirement is
+that the shell be installed on the system.
+
+# - Emacs configuration
+# - Org-mode configuration (org-babel-do-load-languages)
+Once the shell is installed on the system, the Org Babel shell
+language facility must be set to load[fn:4]:
+
+#+begin_example emacs-lisp
+;; active Babel languages
+(org-babel-do-load-languages
+ 'org-babel-load-languages
+ '((shell . t)))
+#+end_example
+
+*NOTE:* This is all that's needed, regardless of the shell you intend
+to use! The "shell" <lang> loads functionality for /all/ supported
+shells.  The car of =(shell . t)=, namely "shell", is the name of the
+Org Babel module that handles it, =ob-shell.el=.
+
+*NOTE:* The =ob-shell.el= module used to be named =ob-sh.el=. This was
+changed in Org 8.2[fn:5].
+
 * Babel Features for Language Source Code Blocks
 ** Header Arguments
    - Language-specific default values
@@ -84,3 +159,27 @@
   - Hello World!
   - Common uses
   - Links to tutorials and other resources
+
+* Footnotes
+
+[fn:1] https://opensource.com/article/19/7/what-posix-richard-stallman-explains
+
+[fn:2] https://en.wikipedia.org/wiki/List_of_GNU_Core_Utilities_commands
+
+[fn:3] https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#index-_0024_0024
+
+[fn:4] https://orgmode.org/worg/org-contrib/babel/languages/index.html#configure
+
+[fn:5] https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/etc/ORG-NEWS#n3995 
+
+#+begin_example
+,* Version 8.2
+
+,** Incompatible changes
+,*** =ob-sh.el= renamed to =ob-shell=
+This may require two changes in user config.
+
+1. In =org-babel-do-load-languages=, change =(sh . t)= to =(shell . t)=.
+2. Edit =local.mk= files to change the value of =BTEST_OB_LANGUAGES=
+   to remove "sh" and include "shell".
+#+end_example
-- 
2.34.0


[-- Attachment #3: 0001-org-contrib-babel-languages-ob-doc-shell.org-Create-.patch --]
[-- Type: application/octet-stream, Size: 4295 bytes --]

From 682eb6bd14d7d7246e3f13517adcd9ffb1a38ae0 Mon Sep 17 00:00:00 2001
From: Matt <matt@excalamus.com>
Date: Mon, 22 Nov 2021 22:57:38 -0500
Subject: [PATCH] org-contrib/babel/languages/ob-doc-shell.org: Create file

* ob-doc-shell.org Create file.  Fill out links and images.
---
 org-contrib/babel/languages/ob-doc-shell.org | 86 ++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 org-contrib/babel/languages/ob-doc-shell.org

diff --git a/org-contrib/babel/languages/ob-doc-shell.org b/org-contrib/babel/languages/ob-doc-shell.org
new file mode 100644
index 00000000..46ce0a6f
--- /dev/null
+++ b/org-contrib/babel/languages/ob-doc-shell.org
@@ -0,0 +1,86 @@
+#+OPTIONS:    H:3 num:nil toc:2 \n:nil ::t |:t ^:{} -:t f:t *:t tex:t d:(HIDE) tags:not-in-toc broken-links:nil
+#+STARTUP:    align fold nodlcheck hidestars oddeven lognotestate hideblocks
+#+SEQ_TODO:   TODO(t) INPROGRESS(i) WAITING(w@) | DONE(d) CANCELED(c@)
+#+TAGS:       Write(w) Update(u) Fix(f) Check(c) noexport(n)
+#+TITLE:      Shell Source Code Blocks in Org Mode
+#+AUTHOR:     Matt Trzcinski
+#+EMAIL:      matt[at]excalamus[dot]com
+#+LANGUAGE:   en
+#+HTML_LINK_UP:    index.html
+#+HTML_LINK_HOME:  https://orgmode.org/worg/
+#+EXCLUDE_TAGS: noexport
+
+#+name: banner
+#+begin_export html
+<div id="subtitle" style="float: center; text-align: center;">
+  <p>
+    Babel shell support, including: <a href="https://www.gnu.org/software/bash/">sh</a>,
+    <a href="https://www.gnu.org/software/bash/">bash</a>,
+    <a href="https://www.zsh.org/">zsh</a>,
+    <a href="https://fishshell.com/">fish</a>,
+    <a href="https://www.grymoire.com/unix/csh.html">csh</a>,
+    <a href="https://www.in-ulm.de/~mascheck/various/ash/">ash</a>,
+    <a href="http://gondor.apana.org.au/~herbert/dash/">dash</a>,
+    <a href="http://www.kornshell.org/">ksh</a>,
+    <a href="https://www.mirbsd.org/mksh.htm">mksh</a>, and
+    <a href="https://packages.qa.debian.org/p/posh.html">posh</a>.
+  </p>
+  <p>
+    <a href="https://www.gnu.org/software/bash/">
+      <img src="https://tiswww.case.edu/php/chet/img/bash-logo-web.png"/>
+    </a>
+    <a href="https://www.zsh.org/">zsh</a>,
+      <img src="https://zsh.sourceforge.io/Images/wizard.gif"/>
+    </a>
+    <a href="https://fishshell.com/">fish</a>,
+      <img src="https://fishshell.com/assets/img/Terminal_Logo2_CRT_Flat.png"/>
+    </a>
+    <a href="http://www.kornshell.org/">ksh</a>,
+      <img src="http://www.kornshell.org/kornshell.gif"/>
+    </a>
+    <a href="https://www.mirbsd.org/mksh.htm">mksh</a>, and
+      <img src="https://www.mirbsd.org/pics/logo-grey.png"/>
+    </a>
+  </p>
+</div>
+#+end_export
+
+* Template Checklist [4/12]                                        :noexport:
+  - [X] Revise #+TITLE:
+  - [X] Indicate #+AUTHOR:
+  - [X] Add #+EMAIL:
+  - [X] Revise banner source block [3/3]
+    - [X] Add link to a useful language web site
+    - [X] Replace "Language" with language name
+    - [X] Find a suitable graphic and use it to link to the language
+      web site
+  - [ ] Write an [[Introduction]]
+  - [ ] Describe [[Requirements and Setup][Requirements and Setup]]
+  - [ ] Replace "Language" with language name in [[Org Mode Features for Language Source Code Blocks][Babel Features for Language Source Code Blocks]]
+  - [ ] Describe [[Header Arguments][Header Arguments]]
+  - [ ] Describe support for [[Sessions]]
+  - [ ] Describe [[Result Types][Result Types]]
+  - [ ] Describe [[Other]] differences from supported languages
+  - [ ] Provide brief [[Examples of Use][Examples of Use]]
+* Introduction
+  - Brief description of language.
+  - Range of typical uses within Org Mode.
+* Requirements and Setup
+  - Installation and configuration of language software
+  - Emacs configuration
+  - Org-mode configuration (org-babel-do-load-languages)
+* Babel Features for Language Source Code Blocks
+** Header Arguments
+   - Language-specific default values
+   - Language-specific header arguments
+** Sessions
+   - Support or not
+   - Typical use for sessions
+** Result Types
+   - Which result types are supported?
+** Other
+   - Differences from other supported languages
+* Examples of Use
+  - Hello World!
+  - Common uses
+  - Links to tutorials and other resources
-- 
2.34.0


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-18  7:03           ` Matt
@ 2021-12-18 20:51             ` Thomas S. Dye
  2021-12-31 17:04             ` Thomas S. Dye
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas S. Dye @ 2021-12-18 20:51 UTC (permalink / raw)
  To: Matt; +Cc: emacs-orgmode, Timothy

Aloha Matt,

Matt <matt@excalamus.com> writes:

>  > Contributions to Worg aren't similarly restricted.  Feel free 
>  > to 
>  > push material there in the meantime.
>
> Looks like the email finally got sent to the right person at my 
> company. Who knows how long it will take for them to get the FSF 
> disclaimer back to me...
>
> Now that I'm on holiday, I've got a little more time to 
> contribute to Worg. Here's a few patches showing what I've come 
> up with. I'll finish it up in the next few days, hopefully.  Not 
> sure if the patches are formed correctly,  sorry about that if 
> not. It's late here and I wanted to show progress. I'll worry 
> about those kinds of details later. :)

Good news.

FYI, you don't need to format patches for Worg.  IIRC, you've 
registered at sr.ht and sent your user name to Bastien, so you 
should be able to push directly to Worg when you're ready.

Thanks for your help with ob-doc-shell.org.

All the best,
Tom
-- 
Thomas S. Dye
https://tsdye.online/tsdye


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-18  7:03           ` Matt
  2021-12-18 20:51             ` Thomas S. Dye
@ 2021-12-31 17:04             ` Thomas S. Dye
  2021-12-31 19:18               ` Matt
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas S. Dye @ 2021-12-31 17:04 UTC (permalink / raw)
  To: Matt; +Cc: emacs-orgmode, Timothy

Aloha Matt,

Wow.  Nice work!

All the best,
Tom

Matt <matt@excalamus.com> writes:

>  > Contributions to Worg aren't similarly restricted.  Feel free 
>  > to 
>  > push material there in the meantime.
>
> Looks like the email finally got sent to the right person at my 
> company. Who knows how long it will take for them to get the FSF 
> disclaimer back to me...
>
> Now that I'm on holiday, I've got a little more time to 
> contribute to Worg. Here's a few patches showing what I've come 
> up with. I'll finish it up in the next few days, hopefully.  Not 
> sure if the patches are formed correctly,  sorry about that if 
> not. It's late here and I wanted to show progress. I'll worry 
> about those kinds of details later. :)


-- 
Thomas S. Dye
https://tsdye.online/tsdye


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-31 17:04             ` Thomas S. Dye
@ 2021-12-31 19:18               ` Matt
  2021-12-31 22:11                 ` Thomas S. Dye
  2022-01-05 17:12                 ` Max Nikulin
  0 siblings, 2 replies; 19+ messages in thread
From: Matt @ 2021-12-31 19:18 UTC (permalink / raw)
  To: Thomas S. Dye; +Cc: emacs-orgmode, Timothy


 > Wow.  Nice work!

Thanks. I pushed things to Worg, if you haven't seen already (https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html). If you read it and find anything missing or unclear, please let me know. I'm still waiting on work to sign the FSF disclaimer in order to work on ob-shell.el. I might just implement something on the side and use that to inform any future contributions.


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-31 19:18               ` Matt
@ 2021-12-31 22:11                 ` Thomas S. Dye
  2022-01-02  4:32                   ` Matt
  2022-01-05 17:12                 ` Max Nikulin
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas S. Dye @ 2021-12-31 22:11 UTC (permalink / raw)
  To: Matt; +Cc: emacs-orgmode, Timothy

Aloha Matt,

Matt <matt@excalamus.com> writes:

>  > Wow.  Nice work!
>
> Thanks. I pushed things to Worg, if you haven't seen already 
> (https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html). 
> If you read it and find anything missing or unclear, please let 
> me know. I'm still waiting on work to sign the FSF disclaimer in 
> order to work on ob-shell.el. I might just implement something 
> on the side and use that to inform any future contributions.

I was looking at the Worg page when I wrote "Wow" :)

Many thanks for this contribution.

All the best,
Tom

-- 
Thomas S. Dye
https://tsdye.online/tsdye


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-31 22:11                 ` Thomas S. Dye
@ 2022-01-02  4:32                   ` Matt
  2022-01-02 18:57                     ` Thomas S. Dye
  0 siblings, 1 reply; 19+ messages in thread
From: Matt @ 2022-01-02  4:32 UTC (permalink / raw)
  To: Thomas S. Dye; +Cc: emacs-orgmode, Timothy

Apologies for the book.  I've been sitting on this stuff for two months and am wondering how to proceed.

IANAL but AFAIK/CT, my contract contains an exception for making contributions to projects like Org. I've gotten confirmation from my manager and by HR.  However, until the CEO signs the FSF disclaimer, I can't officially contribute.  I'm confident that I can publish changes (e.g. to a personal website); the FSF just can't accept my changes (yet).

I could start working on ob-shell.el separately now and publish that myself. It's not clear how I would then contribute those changes back once I'm cleared by the FSF.  I'm inclined towards some refactoring and I'm not sure how I could break that down in such a way that if it took two more months to get the copyright stuff worked out that anyone could make sense of the changes.  I would much prefer to gradually submit patches, discuss them, and then eventually have them merged in turn.  If I have a heap of changes elsewhere, I feel like it would be harder to have a conversion about them.

Regardless, I think I should define test cases first.  If those are considered valid, then any refactoring would be moot if they pass, right?  With (agreed upon) test cases, it shouldn't matter if I refactor as long as functionality remains the same?

Overall, what advice do you have?

It looks to me like ob-shell.el could use some love in other respects besides async evaluation.  I've detailed where below, partly for my own organization and partly for posterity, but mainly because this isn't my house, so to speak, and I don't want to barge in and start rearranging the furniture and eating whatever's in the fridge.  Or, is it like Worg in that once I have the keys I can drive where I like, so long as there're no crashes?

I'm interested in people's thoughts on these notes on ob-shell.el:

* Tests
There are several code paths, like shebang, cmdline, and basic execution, which don't always have something to do with one another in the code.  Having tests would be really helpful to make sure everything jives.  Before doing anything with the code base, I intend to create tests for all the functionality.

* 2D-array
I documented two options for the =:var= header[fn:1]. The ob-shell.el code actually defines a third option for 2D-arrays.  I couldn't get it to work.  It was one of several things not documented anywhere, at least as far as I could find, and which I had to figure out straight from the code.  Between not being great at shell scripting and having a hard time deciphering that ob-shell.el code, I'm not sure 2D-arrays are actually fully or correctly implemented.

* M-up behavior <<M-up>>
The =org-babel-load-session:shell= code path only works when M-up is used on a code block[fn:2]. Is M-up actually documented anywhere?  Furthermore, the =org-babel-load-session:shell= only works for the "shell" language, which is not actually a "proper" shell (i.e. it's not listed in =org-babel-shell-names=). The M-up defaults to using $ESHELL or shell-file-name through the =shell= command.

For example, try calling M-up on these:

#+comment: (opaquely) calls the system shell
#+begin_src shell :session my-shell
echo "hello, world!"  #+end_src

#+comment: fails
#+begin_src sh :session my-sh
echo "hello, world!"  #+end_src

#+comment: fails
#+begin_src bash :session my-bash
echo "hello, world!"  #+end_src

To fix this, there needs to be an =org-babel-load-session:<lang>= for each language in =org-babel-shell-names=.  This would probably make the most sense in =org-babel-shell-initialize=.  However, that function [[org-babel-shell-initialize][needs attention]].

* Refactoring <<refactoring>>
The ob-shell.el code appears inconsistent to me.  Clearly, this is somewhat subjective.  I've tried to give a rationale for each point to make it less so.  My goal is to be maintainer of ob-shell.el, but that's not forever.  These things were an obstacle for me and my aim is to remove them for the next person.

** =org-babel-shell-initialize= <<org-babel-shell-initialize>>
As alluded to elsewhere, =org-babel-shell-initialize= doesn't appear to adhere to the (elisp) Coding Conventions,

#+begin_quote
   • 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.  #+end_quote

The =org-babel-shell-initialize= function defines =org-babel-execute:<lang>=, =org-babel-variable-assignments:<lang>=, and =org-babel-default-header-args:<lang>= for the "languages" in =org-babel-shell-names=.  As it stands, that =org-babel-shell-initialize= is a function does no harm (aside from being confusing by way of straying from convention).  However, if the [[M-up][M-up]] issue is to be resolved, it seems to me that =org-babel-shell-initialize= should be updated to match the convention (i.e. be a macro).

** Organization
Definitions are introduced in different orders.  For example, the =org-babel-shell-initialize= function whose sole purpose is to work with =org-babel-shell-names= is defined before the reader knows what =org-babel-shell-names= is.  Later, this pattern of defining the component pieces after they're used is reversed. For example, =org-babel-load-session:shell= relies on =org-babel-prep-session:shell= which is defined first.  I find defining terms before they're used makes a document more easy to comprehend than otherwise.  I want to reorder the definitions.

Similarly, some functions could use breaking out.  The most important is probably =org-babel-sh-evaluate= which handles the various header arguments.  There are various paths of execution each requiring separate logic, yet all live in the one large function.  Breaking these out would allow them to have separate docstrings and, I expect, be easier to understand since the logic of one would be (lexically) separated from the rest.

Other functionality might be better served by consolidating functions. There's a lot of fiddly code dedicated to variable assignment. Actually, much of the ob-shell.el file is related to variable assignment. The assignments are done using separate functions, yet all apply to the same task. They'll never be used for anything else.  Do they need to be split out?  Is there a technical reason? I don't see one.  Does it help comprehension?  When split out as they are, I found it hard to make sense of the context they're used in since they're defined apart from the logic that uses them (i.e. what header uses them, what form does the header take, etc.).  I think it's worth seeing if there's a better way to present that part of the code base.

** Naming
The following apply to all shells, not just "sh" and should be updated to be "shell". This looks like cruft from when ob-shell.el was called ob-sh.el AFAICT.

- =org-babel-sh-evaluate=
- =org-babel-sh-eoe-indicator=
- =org-babel-sh-eoe-output=
- =org-babel--variable-assignments:sh-generic=
- =org-babel-sh-var-to-sh=
- =org-babel-sh-var-to-string=
- =org-babel-sh-initiate-session=
- =org-babel-sh-evaluate=
- =org-babel-sh-strip-weird-long-prompt=

Generally speaking, I find the Org Babel code base tricky to read (especially at first).  I spent a good deal of time untangling what lived where and who did what. I can play along fine now that I'm familiar. However, since understanding took longer than I think was necessary, I want to detail the pain points as they have made contributing to Babel harder.

Overall, Babel somewhat breaks the (elisp) Coding Conventions for naming,

#+begin_quote
You should choose a short word to distinguish your program from other Lisp programs.  #+end_quote

I understand the variable/function name prefix to be the file name, typically.  The file name is often the package name, or more precisely the feature provided by the file[fn:3]. For Org Babel, there's not a solid file-to-prefix relation.  We say "Org Babel", but the main functionality is in ob-core and the various "ob-" files either extend or implement implied behavior (e.g. =org-babel-<lang>-execute=).  Is the "program" ob-core, ob-lang, or the whole suite of files?  This is a subjective question which the Org Babel "program" answers with, "the whole suite of files".  All components, across all "ob-" files, bear the name "org-babel-". This is still something that trips me up: is the current symbol core or not?  Who is responsible for what?

I would expect the core API to have its own prefix. The extensions would then define their code and have a different prefix, "ob-<lang>-". This way, readers/contributors could open the pertinent ob-* file, see the expected symbol prefix (e.g. "ob-shell-") and another prefix (e.g. "org-babel-") and be able to distinguish which is which.  As it stands, ob-core.el could be renamed to org-babel.el or the "org-babel-" prefix could be changed to "ob-core-".

Another possible solution, or a stopgap, would be to have a document detailing the Org Babel API[fn:4].

* Process interaction
Emacs has several different ways of interacting with processes. The ob-shell.el code uses a few of them. Since async is another way to interact with a process, a single process pattern could be used. The goal would be to make each of the different functionalities provided by ob-shell.el have a similar implementation.  The expectation is that this would benefit maintenance.

* Footnotes

[fn:1] https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html#orgfa6b7c5

[fn:2] M-up is bound to =org-metaup-hook= and
=ob-core:org-babel-load-in-session= by default.

[fn:3] It's not clear to me if there's a technical definition for an
Emacs package.

[fn:4] I may extend my personal notes into a document detailing the
Org API. http://excalamus.com/2021-11-03-org_babel.html



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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2022-01-02  4:32                   ` Matt
@ 2022-01-02 18:57                     ` Thomas S. Dye
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas S. Dye @ 2022-01-02 18:57 UTC (permalink / raw)
  To: Matt; +Cc: emacs-orgmode, Timothy

Aloha all,

FWIW, as a user actively pursuing reproducible research with Org 
and a contributor to documentation about Org and Babel intended 
for other users (rather than Org mode elisp coders) I'd be pleased 
if Org's code custodians look favorably on this proposal.

+1

All the best,
Tom

Matt <matt@excalamus.com> writes:

> Apologies for the book.  I've been sitting on this stuff for two 
> months and am wondering how to proceed.
>
> IANAL but AFAIK/CT, my contract contains an exception for making 
> contributions to projects like Org. I've gotten confirmation 
> from my manager and by HR.  However, until the CEO signs the FSF 
> disclaimer, I can't officially contribute.  I'm confident that I 
> can publish changes (e.g. to a personal website); the FSF just 
> can't accept my changes (yet).
>
> I could start working on ob-shell.el separately now and publish 
> that myself. It's not clear how I would then contribute those 
> changes back once I'm cleared by the FSF.  I'm inclined towards 
> some refactoring and I'm not sure how I could break that down in 
> such a way that if it took two more months to get the copyright 
> stuff worked out that anyone could make sense of the changes.  I 
> would much prefer to gradually submit patches, discuss them, and 
> then eventually have them merged in turn.  If I have a heap of 
> changes elsewhere, I feel like it would be harder to have a 
> conversion about them.
>
> Regardless, I think I should define test cases first.  If those 
> are considered valid, then any refactoring would be moot if they 
> pass, right?  With (agreed upon) test cases, it shouldn't matter 
> if I refactor as long as functionality remains the same?
>
> Overall, what advice do you have?
>
> It looks to me like ob-shell.el could use some love in other 
> respects besides async evaluation.  I've detailed where below, 
> partly for my own organization and partly for posterity, but 
> mainly because this isn't my house, so to speak, and I don't 
> want to barge in and start rearranging the furniture and eating 
> whatever's in the fridge.  Or, is it like Worg in that once I 
> have the keys I can drive where I like, so long as there're no 
> crashes?
>
> I'm interested in people's thoughts on these notes on 
> ob-shell.el:
>
> * Tests
> There are several code paths, like shebang, cmdline, and basic 
> execution, which don't always have something to do with one 
> another in the code.  Having tests would be really helpful to 
> make sure everything jives.  Before doing anything with the code 
> base, I intend to create tests for all the functionality.
>
> * 2D-array
> I documented two options for the =:var= header[fn:1]. The 
> ob-shell.el code actually defines a third option for 2D-arrays. 
> I couldn't get it to work.  It was one of several things not 
> documented anywhere, at least as far as I could find, and which 
> I had to figure out straight from the code.  Between not being 
> great at shell scripting and having a hard time deciphering that 
> ob-shell.el code, I'm not sure 2D-arrays are actually fully or 
> correctly implemented.
>
> * M-up behavior <<M-up>>
> The =org-babel-load-session:shell= code path only works when 
> M-up is used on a code block[fn:2]. Is M-up actually documented 
> anywhere?  Furthermore, the =org-babel-load-session:shell= only 
> works for the "shell" language, which is not actually a "proper" 
> shell (i.e. it's not listed in =org-babel-shell-names=). The 
> M-up defaults to using $ESHELL or shell-file-name through the 
> =shell= command.
>
> For example, try calling M-up on these:
>
> #+comment: (opaquely) calls the system shell
> #+begin_src shell :session my-shell
> echo "hello, world!"  #+end_src
>
> #+comment: fails
> #+begin_src sh :session my-sh
> echo "hello, world!"  #+end_src
>
> #+comment: fails
> #+begin_src bash :session my-bash
> echo "hello, world!"  #+end_src
>
> To fix this, there needs to be an 
> =org-babel-load-session:<lang>= for each language in 
> =org-babel-shell-names=.  This would probably make the most 
> sense in =org-babel-shell-initialize=.  However, that function 
> [[org-babel-shell-initialize][needs attention]].
>
> * Refactoring <<refactoring>>
> The ob-shell.el code appears inconsistent to me.  Clearly, this 
> is somewhat subjective.  I've tried to give a rationale for each 
> point to make it less so.  My goal is to be maintainer of 
> ob-shell.el, but that's not forever.  These things were an 
> obstacle for me and my aim is to remove them for the next 
> person.
>
> ** =org-babel-shell-initialize= <<org-babel-shell-initialize>>
> As alluded to elsewhere, =org-babel-shell-initialize= doesn't 
> appear to adhere to the (elisp) Coding Conventions,
>
> #+begin_quote
>    • 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. 
>    #+end_quote
>
> The =org-babel-shell-initialize= function defines 
> =org-babel-execute:<lang>=, 
> =org-babel-variable-assignments:<lang>=, and 
> =org-babel-default-header-args:<lang>= for the "languages" in 
> =org-babel-shell-names=.  As it stands, that 
> =org-babel-shell-initialize= is a function does no harm (aside 
> from being confusing by way of straying from convention). 
> However, if the [[M-up][M-up]] issue is to be resolved, it seems 
> to me that =org-babel-shell-initialize= should be updated to 
> match the convention (i.e. be a macro).
>
> ** Organization
> Definitions are introduced in different orders.  For example, 
> the =org-babel-shell-initialize= function whose sole purpose is 
> to work with =org-babel-shell-names= is defined before the 
> reader knows what =org-babel-shell-names= is.  Later, this 
> pattern of defining the component pieces after they're used is 
> reversed. For example, =org-babel-load-session:shell= relies on 
> =org-babel-prep-session:shell= which is defined first.  I find 
> defining terms before they're used makes a document more easy to 
> comprehend than otherwise.  I want to reorder the definitions.
>
> Similarly, some functions could use breaking out.  The most 
> important is probably =org-babel-sh-evaluate= which handles the 
> various header arguments.  There are various paths of execution 
> each requiring separate logic, yet all live in the one large 
> function.  Breaking these out would allow them to have separate 
> docstrings and, I expect, be easier to understand since the 
> logic of one would be (lexically) separated from the rest.
>
> Other functionality might be better served by consolidating 
> functions. There's a lot of fiddly code dedicated to variable 
> assignment. Actually, much of the ob-shell.el file is related to 
> variable assignment. The assignments are done using separate 
> functions, yet all apply to the same task. They'll never be used 
> for anything else.  Do they need to be split out?  Is there a 
> technical reason? I don't see one.  Does it help comprehension? 
> When split out as they are, I found it hard to make sense of the 
> context they're used in since they're defined apart from the 
> logic that uses them (i.e. what header uses them, what form does 
> the header take, etc.).  I think it's worth seeing if there's a 
> better way to present that part of the code base.
>
> ** Naming
> The following apply to all shells, not just "sh" and should be 
> updated to be "shell". This looks like cruft from when 
> ob-shell.el was called ob-sh.el AFAICT.
>
> - =org-babel-sh-evaluate=
> - =org-babel-sh-eoe-indicator=
> - =org-babel-sh-eoe-output=
> - =org-babel--variable-assignments:sh-generic=
> - =org-babel-sh-var-to-sh=
> - =org-babel-sh-var-to-string=
> - =org-babel-sh-initiate-session=
> - =org-babel-sh-evaluate=
> - =org-babel-sh-strip-weird-long-prompt=
>
> Generally speaking, I find the Org Babel code base tricky to 
> read (especially at first).  I spent a good deal of time 
> untangling what lived where and who did what. I can play along 
> fine now that I'm familiar. However, since understanding took 
> longer than I think was necessary, I want to detail the pain 
> points as they have made contributing to Babel harder.
>
> Overall, Babel somewhat breaks the (elisp) Coding Conventions 
> for naming,
>
> #+begin_quote
> You should choose a short word to distinguish your program from 
> other Lisp programs.  #+end_quote
>
> I understand the variable/function name prefix to be the file 
> name, typically.  The file name is often the package name, or 
> more precisely the feature provided by the file[fn:3]. For Org 
> Babel, there's not a solid file-to-prefix relation.  We say "Org 
> Babel", but the main functionality is in ob-core and the various 
> "ob-" files either extend or implement implied behavior (e.g. 
> =org-babel-<lang>-execute=).  Is the "program" ob-core, ob-lang, 
> or the whole suite of files?  This is a subjective question 
> which the Org Babel "program" answers with, "the whole suite of 
> files".  All components, across all "ob-" files, bear the name 
> "org-babel-". This is still something that trips me up: is the 
> current symbol core or not?  Who is responsible for what?
>
> I would expect the core API to have its own prefix. The 
> extensions would then define their code and have a different 
> prefix, "ob-<lang>-". This way, readers/contributors could open 
> the pertinent ob-* file, see the expected symbol prefix (e.g. 
> "ob-shell-") and another prefix (e.g. "org-babel-") and be able 
> to distinguish which is which.  As it stands, ob-core.el could 
> be renamed to org-babel.el or the "org-babel-" prefix could be 
> changed to "ob-core-".
>
> Another possible solution, or a stopgap, would be to have a 
> document detailing the Org Babel API[fn:4].
>
> * Process interaction
> Emacs has several different ways of interacting with processes. 
> The ob-shell.el code uses a few of them. Since async is another 
> way to interact with a process, a single process pattern could 
> be used. The goal would be to make each of the different 
> functionalities provided by ob-shell.el have a similar 
> implementation.  The expectation is that this would benefit 
> maintenance.
>
> * Footnotes
>
> [fn:1] 
> https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html#orgfa6b7c5
>
> [fn:2] M-up is bound to =org-metaup-hook= and
> =ob-core:org-babel-load-in-session= by default.
>
> [fn:3] It's not clear to me if there's a technical definition 
> for an
> Emacs package.
>
> [fn:4] I may extend my personal notes into a document detailing 
> the
> Org API. http://excalamus.com/2021-11-03-org_babel.html


-- 
Thomas S. Dye
https://tsdye.online/tsdye


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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2021-12-31 19:18               ` Matt
  2021-12-31 22:11                 ` Thomas S. Dye
@ 2022-01-05 17:12                 ` Max Nikulin
  2022-01-06  3:47                   ` Matt
  1 sibling, 1 reply; 19+ messages in thread
From: Max Nikulin @ 2022-01-05 17:12 UTC (permalink / raw)
  To: emacs-orgmode

On 01/01/2022 02:18, Matt wrote:
> 
>   > Wow.  Nice work!
> 
> Thanks. I pushed things to Worg, if you haven't seen already
> (https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html).
> If you read it and find anything missing or unclear, please let me
> know.

Thank you, I was not aware of ":cmdline" argument and of ":shebang" as a 
means to avoid stray prompts (I have seen mentions of similar problem 
for other languages in this list).

I have noticed some glitches.

- https://www.mirbsd.org/pics/logo-grey.png works only with http: 
protocol for me.
- I am unsure concerning example after "…or as a standlone script.", 
maybe it should be wrapped into #+begin_example.
- In some cases "sh" is used despite bashisms in the code like "declare" 
or "echo -ne". Actually "printf" may be more convenient instead of "echo -n"
- "export" in session example is unnecessary. It is matter of taste 
though. Bash and dash is not the case, but some shells require that 
assignment and export should be separate commands.
- In your examples variable values are simple. Often it is safer to add 
double quotes around variable or command expansion. I would consider 
adding quotes just to encourage people to do the same by default with 
hope that less scripts will give strange results in response to a file 
name containing a space. Actually my first impression was that 
backslashes before quotes in some cases were added by mistake. Another 
unescaped pair of quotes may make your intention more clear. However it 
is related to code style where everybody has opinion, so I do not insist.





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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2022-01-05 17:12                 ` Max Nikulin
@ 2022-01-06  3:47                   ` Matt
  2022-01-07 16:18                     ` Max Nikulin
  0 siblings, 1 reply; 19+ messages in thread
From: Matt @ 2022-01-06  3:47 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

 > Thank you, I was not aware of ":cmdline" argument and of ":shebang" as a 
 > means to avoid stray prompts (I have seen mentions of similar problem 
 > for other languages in this list).
 
You're welcome! I was surprised to find those in the source. Glad to have documented them and even happier to hear that it's news to others.
 
 > - https://www.mirbsd.org/pics/logo-grey.png works only with http: 
 > protocol for me.

I updated (downgraded?) it to http.  I'm also seeing that the kornshell logo is intermittently broken.  Not quite sure the best way to go about fixing that.   Maybe commit copies?  Use something like archive.org? Sounds like a problem for someone else (e.g. future me).  I'm out of time for tonight.

 > - I am unsure concerning example after "…or as a standlone script.", 
 > maybe it should be wrapped into #+begin_example.

Good call.  That example was wonky.  I've given a different one.  Not sure it's better, though.  I find the shebang behavior tricky to demo in a simple way.

 > - In some cases "sh" is used despite bashisms in the code like "declare" 
 > or "echo -ne". Actually "printf" may be more convenient instead of "echo -n"
 > - "export" in session example is unnecessary. It is matter of taste 
 > though. Bash and dash is not the case, but some shells require that 
 > assignment and export should be separate commands.

Good catch!  Since arrays are only defined in ob-shell using bash, I've updated that example to use bash. Otherwise, I've removed the exports. 

 > - In your examples variable values are simple. Often it is safer to add 
 > double quotes around variable or command expansion. I would consider 
 > adding quotes just to encourage people to do the same by default with 
 > hope that less scripts will give strange results in response to a file 
 > name containing a space. Actually my first impression was that 
 > backslashes before quotes in some cases were added by mistake. Another 
 > unescaped pair of quotes may make your intention more clear. However it 
 > is related to code style where everybody has opinion, so I do not insist.

I appreciate these clarifications. Admittedly, I'm not great with shell scripting.  Your recommendations appear sound to me and I've tried to incorporate them.

The updates have been made and are pushed. Thanks for your feedback!



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

* Re: [PATCH] ob-shell-test, test-ob-shell and introduction
  2022-01-06  3:47                   ` Matt
@ 2022-01-07 16:18                     ` Max Nikulin
  0 siblings, 0 replies; 19+ messages in thread
From: Max Nikulin @ 2022-01-07 16:18 UTC (permalink / raw)
  To: emacs-orgmode

On 06/01/2022 10:47, Matt wrote:
> 
>   > - In your examples variable values are simple. Often it is safer to add
>   > double quotes around variable or command expansion. I would consider
>   > adding quotes just to encourage people to do the same by default with
> 
> I appreciate these clarifications. Admittedly, I'm not great with
> shell scripting.  Your recommendations appear sound to me and I've tried
> to incorporate them.

I think, to illustrate unset variable escaped quotes were appropriate. 
Sorry that I was not clear enough.

    echo "X was set to \"$X\""

One more case where it is better (at least from my point of view) to add 
quotes even though they are not strictly necessary since command output 
contains single word and multiple words are interpreted by echo in the 
same way:

    echo "$(cut -f 1 -d "/") rocks!"

> The updates have been made and are pushed. Thanks for your feedback!

Thank you, text is more clear now. However first time I read it with 
more attention.

I forgot to suggest you the following tool that catches more problems 
than execution the script with "-n" option:

echo 'a=2; echo "$(($a * 2))"' | shellcheck -s sh -

In - line 1:
a=2; echo "$(($a * 2))"
               ^-- SC2004: $/${} is unnecessary on arithmetic variables.

For more information:
   https://www.shellcheck.net/wiki/SC2004 -- $/${} is unnecessary on 
arithmeti...



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

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

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 18:37 [PATCH] ob-shell-test, test-ob-shell and introduction Matt
2021-11-22 18:43 ` Timothy
2021-11-24 18:48   ` Matt
2021-12-02  9:39     ` Timothy
2021-12-06  3:50       ` Matt
2021-12-06  4:50         ` Thomas S. Dye
2021-12-18  7:03           ` Matt
2021-12-18 20:51             ` Thomas S. Dye
2021-12-31 17:04             ` Thomas S. Dye
2021-12-31 19:18               ` Matt
2021-12-31 22:11                 ` Thomas S. Dye
2022-01-02  4:32                   ` Matt
2022-01-02 18:57                     ` Thomas S. Dye
2022-01-05 17:12                 ` Max Nikulin
2022-01-06  3:47                   ` Matt
2022-01-07 16:18                     ` Max Nikulin
2021-11-22 19:08 ` Thomas S. Dye
2021-11-23  4:42   ` [PATCH] ob-doc-shell.org (was [PATCH] ob-shell-test, test-ob-shell and introduction) Matt
2021-11-23  4:59     ` Thomas S. Dye

Code repositories for project(s) associated with this 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).