From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id cPeZA8X10WHqpgAAgWs5BA (envelope-from ) for ; Sun, 02 Jan 2022 19:58:13 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id 8PL4N8T10WFbgAEAG6o9tA (envelope-from ) for ; Sun, 02 Jan 2022 19:58:12 +0100 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 4B29EEA29 for ; Sun, 2 Jan 2022 19:58:12 +0100 (CET) Received: from localhost ([::1]:40196 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n463X-0007Tz-4T for larch@yhetil.org; Sun, 02 Jan 2022 13:58:11 -0500 Received: from eggs.gnu.org ([209.51.188.92]:58600) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n4635-0007To-NH for emacs-orgmode@gnu.org; Sun, 02 Jan 2022 13:57:43 -0500 Received: from gproxy2-pub.mail.unifiedlayer.com ([69.89.18.3]:41377) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n4633-0006XP-3w for emacs-orgmode@gnu.org; Sun, 02 Jan 2022 13:57:43 -0500 Received: from cmgw14.mail.unifiedlayer.com (unknown [10.0.90.129]) by progateway4.mail.pro1.eigbox.com (Postfix) with ESMTP id 421EC10048661 for ; Sun, 2 Jan 2022 18:57:28 +0000 (UTC) Received: from box2035.bluehost.com ([74.220.219.237]) by cmsmtp with ESMTP id 462pnMIaa2s5d462pnz87O; Sun, 02 Jan 2022 18:57:28 +0000 X-Authority-Reason: nr=8 X-Authority-Analysis: v=2.4 cv=GKvNrsBK c=1 sm=1 tr=0 ts=61d1f598 a=VozZY++RX3oc2UgfNhVfaA==:117 a=VozZY++RX3oc2UgfNhVfaA==:17 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=IkcTkHD0fZMA:10:nop_charset_1 a=MKtGQD3n3ToA:10:nop_fastflux_from_domain_1 a=1oJP67jkp3AA:10:nop_fastflux_mid_domain_1 a=DghFqjY3_ZEA:10:nop_rcvd_month_year a=DPR-AOO6AYYA:10:endurance_base64_authed_username_1 a=A2tt7buDTgEA:10:from_fastflux_domain1 a=ZZnuYtJkoWoA:10:nop_header_rescan a=wk1nm1k_AAAA:8 a=NjIK7znJAAAA:8 a=o9zw6IYYAAAA:8 a=FRoVgTYMHyk8V9Q3zK4A:9 a=QEXdDO2ut3YA:10:nop_charset_2 a=wW9FBh6_r7wA:10:demote_hacked_domain_1 a=RqTjG1EEdD4A:10:demote_hacked_domain_1 a=-FEs8UIgK8oA:10:nop_fastflux_domain_1 a=NWVoK91CQyQA:10:nop_rescan a=x-TXmAUBizrG4PPsTVy8:22 a=O1c1nmEkCxIQdeELtU9r:22 a=BtxB1_lq3pBo68oZtZ_9:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tsdye.online; s=default; h=Content-Transfer-Encoding:Content-Type: MIME-Version:Message-ID:Date:In-reply-to:Subject:Cc:To:From:References:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=U7ELKmebHBt0U/KOSA9xP3EoYR0kbHjM0Jc1aoi9+PA=; b=X/zYjRSX3KBL942OWv9bsMnnD5 7XrpqdRZ+2PY56ym4ktFYkrvaRqd+JktRm1e60GrXcOcvMBaF9+MvnLmP33paJ0w+bsM2X88NOc3T BySaasEbea9GHe2EryWIzNJDs4txCaHvznjGNC9n+slUx3ycUtNEtjnEEPnLKOWd4n9AiC/JJsAJU 0HWKubxF+pJi1NphbK4YtkWKQyDjQADShoL4j5EeDRzzvrt2zc8OftA+GoCA38mjQuAZ8m5EMtByd 9FD23DIxdO7H4r3UzvuMR4PpGnyuy3e5tdhhzuZ7fzeAStFyIF0FK3x3N/HQ07gn+wRHdzl+Usw3R z+zxO4hQ==; Received: from cpe-50-113-36-248.hawaii.res.rr.com ([50.113.36.248]:48420 helo=poto-foou.tsdye.online) by box2035.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1n462p-002vzv-Hp; Sun, 02 Jan 2022 11:57:27 -0700 References: <17d48f09729.ddbece82213631.8340272801066201269@excalamus.com> <87pmqs58mx.fsf@gmail.com> <17d53474fd6.c0a62421660695.6561002670353136607@excalamus.com> <87lf13pcme.fsf@gmail.com> <17d8ddd76f7.d167f605179643.3635445903460764440@excalamus.com> <87bl1u49ru.fsf@tsdye.online> <17dcc5a0e3e.d8ca0ef6361712.4976360594738663013@excalamus.com> <875yr4iu3a.fsf@tsdye.online> <17e11edb77e.adb20ab5289948.958495178633735939@excalamus.com> <8735m8ifvr.fsf@tsdye.online> <17e190f0110.dffbf4bc1032.6816869643164380190@excalamus.com> User-agent: mu4e 1.2.0; emacs 27.1 From: "Thomas S. Dye" To: Matt Subject: Re: [PATCH] ob-shell-test, test-ob-shell and introduction In-reply-to: <17e190f0110.dffbf4bc1032.6816869643164380190@excalamus.com> Date: Sun, 02 Jan 2022 08:57:25 -1000 Message-ID: <87tuemge3u.fsf@tsdye.online> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box2035.bluehost.com X-AntiAbuse: Original Domain - gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tsdye.online X-BWhitelist: no X-Source-IP: 50.113.36.248 X-Source-L: No X-Exim-ID: 1n462p-002vzv-Hp X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: cpe-50-113-36-248.hawaii.res.rr.com (poto-foou.tsdye.online) [50.113.36.248]:48420 X-Source-Auth: tsd@tsdye.online X-Email-Count: 2 X-Source-Cap: dHNkeWVvbmw7dHNkeWVvbmw7Ym94MjAzNS5ibHVlaG9zdC5jb20= X-Local-Domain: yes Received-SPF: pass client-ip=69.89.18.3; envelope-from=tsd@tsdye.online; helo=gproxy2-pub.mail.unifiedlayer.com X-Spam_score_int: 23 X-Spam_score: 2.3 X-Spam_bar: ++ X-Spam_report: (2.3 / 5.0 requ) DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FROM_SUSPICIOUS_NTLD=0.499, PDS_OTHER_BAD_TLD=1.999, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_BL=0.001, RCVD_IN_MSPIKE_L3=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emacs-orgmode , Timothy Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1641149892; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=U7ELKmebHBt0U/KOSA9xP3EoYR0kbHjM0Jc1aoi9+PA=; b=Bu/3KGOh9li7t1KC57jhYFCKvsftnvqmLhDK6BXjVcR9kTIoFyqVCLLiVuHB5ab/XIFWOC kshMUgbeaIopYwxxtrwRWkj309CADfQ93c3v62ci3MnETHs0nxSZdlwIv64cuF8zzpzeka UnJi+hXONf/Kp8WdkN5/SiDcS5BOHbafn4OLFyWqBo9g8n9jOiHMd+EZrakOfngE43/7Oe VSWczMf/q3dS2UeVfdFjJqRXT452BC7KsHO/hXd4o4A2Dw2UkbvdpsP+XyW/L8wQvlECZ2 mKGhWJTAhM6YpZw1LTETHHbKG5r2woRbVqsMlg4fSUhzpxt0TSCJ5B5l4S0+FQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1641149892; a=rsa-sha256; cv=none; b=Oe0K5GN0zap5rJtUvvmkRS0kW1mBXnNE/akE4SKY9N6DTtMZr/EU4l/OHqTdsjrAXozIlN bRx4GyypZ3+4IQ4j0ka7VLWS/tM5E3NiuWw6h1N/txGzhttjK3xMgALi7qMwqsa6ZNGtJO TEL4R79VRqwpru1qmPjc8mFFp/JDFfZqPl+DWPt95PWRWvYZ+Kb+ds9mpR52uOql1L76VC PAIDM8N16jTytiQQSqESXDyeWzeSvNLTLT/LOrJwXyX6+CrG04xmteF07lvDyn1qvzm3oL eeu709/O9S+LRhAHdXC9w8u1BKCuNyF6Wmz786Z4e987YQQ7XQDNQhgYQs6Vaw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=tsdye.online header.s=default header.b="X/zYjRSX"; dmarc=none; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -4.08 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=tsdye.online header.s=default header.b="X/zYjRSX"; dmarc=none; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 4B29EEA29 X-Spam-Score: -4.08 X-Migadu-Scanner: scn0.migadu.com X-TUID: n07Whqi1m70b Aloha all, FWIW, as a user actively pursuing reproducible research with Org=20 and a contributor to documentation about Org and Babel intended=20 for other users (rather than Org mode elisp coders) I'd be pleased=20 if Org's code custodians look favorably on this proposal. +1 All the best, Tom Matt writes: > Apologies for the book. I've been sitting on this stuff for two=20 > months and am wondering how to proceed. > > IANAL but AFAIK/CT, my contract contains an exception for making=20 > contributions to projects like Org. I've gotten confirmation=20 > from my manager and by HR. However, until the CEO signs the FSF=20 > disclaimer, I can't officially contribute. I'm confident that I=20 > can publish changes (e.g. to a personal website); the FSF just=20 > can't accept my changes (yet). > > I could start working on ob-shell.el separately now and publish=20 > that myself. It's not clear how I would then contribute those=20 > changes back once I'm cleared by the FSF. I'm inclined towards=20 > some refactoring and I'm not sure how I could break that down in=20 > such a way that if it took two more months to get the copyright=20 > stuff worked out that anyone could make sense of the changes. I=20 > would much prefer to gradually submit patches, discuss them, and=20 > then eventually have them merged in turn. If I have a heap of=20 > changes elsewhere, I feel like it would be harder to have a=20 > conversion about them. > > Regardless, I think I should define test cases first. If those=20 > are considered valid, then any refactoring would be moot if they=20 > pass, right? With (agreed upon) test cases, it shouldn't matter=20 > 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=20 > respects besides async evaluation. I've detailed where below,=20 > partly for my own organization and partly for posterity, but=20 > mainly because this isn't my house, so to speak, and I don't=20 > want to barge in and start rearranging the furniture and eating=20 > whatever's in the fridge. Or, is it like Worg in that once I=20 > have the keys I can drive where I like, so long as there're no=20 > crashes? > > I'm interested in people's thoughts on these notes on=20 > ob-shell.el: > > * Tests > There are several code paths, like shebang, cmdline, and basic=20 > execution, which don't always have something to do with one=20 > another in the code. Having tests would be really helpful to=20 > make sure everything jives. Before doing anything with the code=20 > base, I intend to create tests for all the functionality. > > * 2D-array > I documented two options for the =3D:var=3D header[fn:1]. The=20 > ob-shell.el code actually defines a third option for 2D-arrays.=20 > I couldn't get it to work. It was one of several things not=20 > documented anywhere, at least as far as I could find, and which=20 > I had to figure out straight from the code. Between not being=20 > great at shell scripting and having a hard time deciphering that=20 > ob-shell.el code, I'm not sure 2D-arrays are actually fully or=20 > correctly implemented. > > * M-up behavior <> > The =3Dorg-babel-load-session:shell=3D code path only works when=20 > M-up is used on a code block[fn:2]. Is M-up actually documented=20 > anywhere? Furthermore, the =3Dorg-babel-load-session:shell=3D only=20 > works for the "shell" language, which is not actually a "proper"=20 > shell (i.e. it's not listed in =3Dorg-babel-shell-names=3D). The=20 > M-up defaults to using $ESHELL or shell-file-name through the=20 > =3Dshell=3D 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=20 > =3Dorg-babel-load-session:=3D for each language in=20 > =3Dorg-babel-shell-names=3D. This would probably make the most=20 > sense in =3Dorg-babel-shell-initialize=3D. However, that function=20 > [[org-babel-shell-initialize][needs attention]]. > > * Refactoring <> > The ob-shell.el code appears inconsistent to me. Clearly, this=20 > is somewhat subjective. I've tried to give a rationale for each=20 > point to make it less so. My goal is to be maintainer of=20 > ob-shell.el, but that's not forever. These things were an=20 > obstacle for me and my aim is to remove them for the next=20 > person. > > ** =3Dorg-babel-shell-initialize=3D <> > As alluded to elsewhere, =3Dorg-babel-shell-initialize=3D doesn't=20 > appear to adhere to the (elisp) Coding Conventions, > > #+begin_quote > =E2=80=A2 Constructs that define a function or variable should be=20 > macros, not functions, and their names should start with=20 > =E2=80=98define-=E2=80=99. The macro should receive the name to be de= fined=20 > as the first argument. That will help various tools find the=20 > definition automatically. Avoid constructing the names in=20 > the macro itself, since that would confuse these tools.=20 > #+end_quote > > The =3Dorg-babel-shell-initialize=3D function defines=20 > =3Dorg-babel-execute:=3D,=20 > =3Dorg-babel-variable-assignments:=3D, and=20 > =3Dorg-babel-default-header-args:=3D for the "languages" in=20 > =3Dorg-babel-shell-names=3D. As it stands, that=20 > =3Dorg-babel-shell-initialize=3D is a function does no harm (aside=20 > from being confusing by way of straying from convention).=20 > However, if the [[M-up][M-up]] issue is to be resolved, it seems=20 > to me that =3Dorg-babel-shell-initialize=3D should be updated to=20 > match the convention (i.e. be a macro). > > ** Organization > Definitions are introduced in different orders. For example,=20 > the =3Dorg-babel-shell-initialize=3D function whose sole purpose is=20 > to work with =3Dorg-babel-shell-names=3D is defined before the=20 > reader knows what =3Dorg-babel-shell-names=3D is. Later, this=20 > pattern of defining the component pieces after they're used is=20 > reversed. For example, =3Dorg-babel-load-session:shell=3D relies on=20 > =3Dorg-babel-prep-session:shell=3D which is defined first. I find=20 > defining terms before they're used makes a document more easy to=20 > comprehend than otherwise. I want to reorder the definitions. > > Similarly, some functions could use breaking out. The most=20 > important is probably =3Dorg-babel-sh-evaluate=3D which handles the=20 > various header arguments. There are various paths of execution=20 > each requiring separate logic, yet all live in the one large=20 > function. Breaking these out would allow them to have separate=20 > docstrings and, I expect, be easier to understand since the=20 > logic of one would be (lexically) separated from the rest. > > Other functionality might be better served by consolidating=20 > functions. There's a lot of fiddly code dedicated to variable=20 > assignment. Actually, much of the ob-shell.el file is related to=20 > variable assignment. The assignments are done using separate=20 > functions, yet all apply to the same task. They'll never be used=20 > for anything else. Do they need to be split out? Is there a=20 > technical reason? I don't see one. Does it help comprehension?=20 > When split out as they are, I found it hard to make sense of the=20 > context they're used in since they're defined apart from the=20 > logic that uses them (i.e. what header uses them, what form does=20 > the header take, etc.). I think it's worth seeing if there's a=20 > better way to present that part of the code base. > > ** Naming > The following apply to all shells, not just "sh" and should be=20 > updated to be "shell". This looks like cruft from when=20 > ob-shell.el was called ob-sh.el AFAICT. > > - =3Dorg-babel-sh-evaluate=3D > - =3Dorg-babel-sh-eoe-indicator=3D > - =3Dorg-babel-sh-eoe-output=3D > - =3Dorg-babel--variable-assignments:sh-generic=3D > - =3Dorg-babel-sh-var-to-sh=3D > - =3Dorg-babel-sh-var-to-string=3D > - =3Dorg-babel-sh-initiate-session=3D > - =3Dorg-babel-sh-evaluate=3D > - =3Dorg-babel-sh-strip-weird-long-prompt=3D > > Generally speaking, I find the Org Babel code base tricky to=20 > read (especially at first). I spent a good deal of time=20 > untangling what lived where and who did what. I can play along=20 > fine now that I'm familiar. However, since understanding took=20 > longer than I think was necessary, I want to detail the pain=20 > points as they have made contributing to Babel harder. > > Overall, Babel somewhat breaks the (elisp) Coding Conventions=20 > for naming, > > #+begin_quote > You should choose a short word to distinguish your program from=20 > other Lisp programs. #+end_quote > > I understand the variable/function name prefix to be the file=20 > name, typically. The file name is often the package name, or=20 > more precisely the feature provided by the file[fn:3]. For Org=20 > Babel, there's not a solid file-to-prefix relation. We say "Org=20 > Babel", but the main functionality is in ob-core and the various=20 > "ob-" files either extend or implement implied behavior (e.g.=20 > =3Dorg-babel--execute=3D). Is the "program" ob-core, ob-lang,=20 > or the whole suite of files? This is a subjective question=20 > which the Org Babel "program" answers with, "the whole suite of=20 > files". All components, across all "ob-" files, bear the name=20 > "org-babel-". This is still something that trips me up: is the=20 > current symbol core or not? Who is responsible for what? > > I would expect the core API to have its own prefix. The=20 > extensions would then define their code and have a different=20 > prefix, "ob--". This way, readers/contributors could open=20 > the pertinent ob-* file, see the expected symbol prefix (e.g.=20 > "ob-shell-") and another prefix (e.g. "org-babel-") and be able=20 > to distinguish which is which. As it stands, ob-core.el could=20 > be renamed to org-babel.el or the "org-babel-" prefix could be=20 > changed to "ob-core-". > > Another possible solution, or a stopgap, would be to have a=20 > document detailing the Org Babel API[fn:4]. > > * Process interaction > Emacs has several different ways of interacting with processes.=20 > The ob-shell.el code uses a few of them. Since async is another=20 > way to interact with a process, a single process pattern could=20 > be used. The goal would be to make each of the different=20 > functionalities provided by ob-shell.el have a similar=20 > implementation. The expectation is that this would benefit=20 > maintenance. > > * Footnotes > > [fn:1]=20 > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html#or= gfa6b7c5 > > [fn:2] M-up is bound to =3Dorg-metaup-hook=3D and > =3Dob-core:org-babel-load-in-session=3D by default. > > [fn:3] It's not clear to me if there's a technical definition=20 > for an > Emacs package. > > [fn:4] I may extend my personal notes into a document detailing=20 > the > Org API. http://excalamus.com/2021-11-03-org_babel.html --=20 Thomas S. Dye https://tsdye.online/tsdye