From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <emacs-orgmode-bounces+larch=yhetil.org@gnu.org>
Received: from mp12.migadu.com ([2001:41d0:2:4a6f::])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	by ms5.migadu.com with LMTPS
	id cIqmMVxgsmOJggAAbAwnHQ
	(envelope-from <emacs-orgmode-bounces+larch=yhetil.org@gnu.org>)
	for <larch@yhetil.org>; Mon, 02 Jan 2023 05:41:00 +0100
Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	by mp12.migadu.com with LMTPS
	id AI/LMVxgsmOT6QAAauVa8A
	(envelope-from <emacs-orgmode-bounces+larch=yhetil.org@gnu.org>)
	for <larch@yhetil.org>; Mon, 02 Jan 2023 05:41:00 +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 D6AFD3D8CA
	for <larch@yhetil.org>; Mon,  2 Jan 2023 05:40:59 +0100 (CET)
Received: from localhost ([::1] helo=lists1p.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.90_1)
	(envelope-from <emacs-orgmode-bounces@gnu.org>)
	id 1pCCcL-0005ZK-MA; Sun, 01 Jan 2023 23:40:09 -0500
Received: from eggs.gnu.org ([2001:470:142:3::10])
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <matt@excalamus.com>)
 id 1pCCcJ-0005Z7-Un
 for emacs-orgmode@gnu.org; Sun, 01 Jan 2023 23:40:07 -0500
Received: from sender4-op-o12.zoho.com ([136.143.188.12])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <matt@excalamus.com>)
 id 1pCCcI-0006WW-05
 for emacs-orgmode@gnu.org; Sun, 01 Jan 2023 23:40:07 -0500
ARC-Seal: i=1; a=rsa-sha256; t=1672634401; cv=none; 
 d=zohomail.com; s=zohoarc; 
 b=hhdwwa+Xz+OnxAeMeyVhjVPnkSgG+zv8A4WUD2z82hjeJ5NMxuVWanNHdU4eXTxEUoymClMCe92UwQF+lPke9CCM0dR7dDLTq6SJVkHAw4Yg4V+wTw+gZPcMcs0oE/BuSq2r8mt1c2NFhb6dXw6PWgteSO8BumUWxsDE90odJ/w=
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com;
 s=zohoarc; t=1672634401;
 h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To;
 bh=4e/O18N8x8v8GwmQboJG5Hx7F8BU+2ww4bo/V3EDong=; 
 b=Qf0EDF7lACq2ONQTemfj5Vtcp1gTS+biz4IUueImFmhp/94ElpG5JOPBx/FBOWWzgbItYSBMlIvAeXzQAZdozwoHR8/y/YP6d11fy52SqnfA9JgDmwNSVbcO0q/R5dI99sxm9HPPkSh+T57CQpxlL0ffrYxpySsStyF/JD2/+N0=
ARC-Authentication-Results: i=1; mx.zohomail.com;
 dkim=pass  header.i=excalamus.com;
 spf=pass  smtp.mailfrom=matt@excalamus.com;
 dmarc=pass header.from=<matt@excalamus.com>
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1672634401; 
 s=zmail; d=excalamus.com; i=matt@excalamus.com;
 h=Date:Date:From:From:To:To:Cc:Cc:Message-ID:In-Reply-To:References:Subject:Subject:MIME-Version:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To;
 bh=4e/O18N8x8v8GwmQboJG5Hx7F8BU+2ww4bo/V3EDong=;
 b=T7aLh36f20PpFWGXbgk+OUJyKQanunFhc01UNixT1wQrBCgCvalLIGGMawq8CVVx
 lVZzD+9yXMSCuGFeS13C8jlXU8im/uBPEi/R42qSi5Fb5uBoq8GWgoreDe3C3bWtEnH
 /4DA0MRwFMRICWjTN0Wf661bS0kna5kQu/TVDuzI=
Received: from mail.zoho.com by mx.zohomail.com
 with SMTP id 1672634400459828.9856732624905;
 Sun, 1 Jan 2023 20:40:00 -0800 (PST)
Date: Sun, 01 Jan 2023 23:40:00 -0500
From: Matt <matt@excalamus.com>
To: "Ihor Radchenko" <yantar92@posteo.net>
Cc: "emacs-orgmode" <emacs-orgmode@gnu.org>
Message-ID: <18570c77eb9.119e6407e40169.7047396916380100893@excalamus.com>
In-Reply-To: <87r0wfafzu.fsf@localhost>
References: <m2tu1v8gj8.fsf@me.com> <878rj7s0ti.fsf@localhost>
 <1853354beb4.fce54d8d902653.6359367327256505471@excalamus.com>
 <18555580d75.d7b471f9370716.1497263973038999899@excalamus.com>
 <87edsiv54k.fsf@localhost>
 <18561866f8c.f22a62b3866025.8634474263936272339@excalamus.com>
 <87r0wfafzu.fsf@localhost>
Subject: Refactor org-babel-shell-initialize? (was Re: ob-shell intentions
 and paperwork (was Bash results broken?))
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Importance: Medium
User-Agent: Zoho Mail
X-Mailer: Zoho Mail
Received-SPF: pass client-ip=136.143.188.12; envelope-from=matt@excalamus.com;
 helo=sender4-op-o12.zoho.com
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,
 RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001,
 SPF_PASS=-0.001 autolearn=ham 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." <emacs-orgmode.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/emacs-orgmode>,
 <mailto:emacs-orgmode-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/emacs-orgmode>
List-Post: <mailto:emacs-orgmode@gnu.org>
List-Help: <mailto:emacs-orgmode-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/emacs-orgmode>,
 <mailto:emacs-orgmode-request@gnu.org?subject=subscribe>
Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org
Sender: emacs-orgmode-bounces+larch=yhetil.org@gnu.org
X-Migadu-Country: US
X-Migadu-Flow: FLOW_IN
ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org;
	s=key1; t=1672634460;
	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=4e/O18N8x8v8GwmQboJG5Hx7F8BU+2ww4bo/V3EDong=;
	b=OpBC1CHjG/2HbhEcpfEQQtI6mhWKO+NGO5r8+iLAKNQL71E8AUU0ocjW9PS0u0Sx5Mewps
	GA8m8gq1dnZgMYZzcfmh0V/toJjzHIEft1OM2FkXEYPuJPpguj+VJwkpBjaP6dY0QzfKtZ
	BW+bCfgXBbzP2Ba+MisgLlf1lJtEFwHefM4Im1TGQP29WgSoxQoo/gUQepWoeiYwIkNoMb
	ulL6Vsz9bcA5+SNyK5fMRvh8lPiqlbb5qax40yRrzKhK9+IiDDQL0VRp3JapDpO8nTukm6
	hByee1hUrXLhbgtHu34vCiEWDuKCB+vyj88L8Lh2wbQGF85LTUerggVA9B2KWw==
ARC-Authentication-Results: i=2;
	aspmx1.migadu.com;
	dkim=pass header.d=excalamus.com header.s=zmail header.b=T7aLh36f;
	arc=pass ("zohomail.com:s=zohoarc:i=1");
	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";
	dmarc=none
ARC-Seal: i=2; s=key1; d=yhetil.org; t=1672634460; a=rsa-sha256; cv=pass;
	b=M8TfTKqojJc8DrJk9tmpEQhwgtrFpSaYagLxQO3YdZ5mQqK6SZ1Dr5BxGSZZcNmAUym0K4
	eQNAwGIIjz4h3GpBkexEzzF3TaLSFIaSpGKa1LMEpFe7VtAsgntoGBlFzuCAXB4W08ZiNq
	wW0b0xvny8TZNSibKmgrM9RIGb/SdA8/2wh2lDdO58pL2NQubsqXFJHTc1Pi8bFS5ey+yP
	yX2U9eRzVQ3sAZOBs225L3QGCheVslb/nLqIVVLOzSe6EzqM4798b3/8k6CmQPFrhRygs3
	JBqFiqeP5DqdZdU6oLSERfyJB6n+mqMPHtIF8pjZ7QBmqV7XXSy4FowmvRC/pQ==
X-Spam-Score: -6.75
X-Migadu-Queue-Id: D6AFD3D8CA
Authentication-Results: aspmx1.migadu.com;
	dkim=pass header.d=excalamus.com header.s=zmail header.b=T7aLh36f;
	arc=pass ("zohomail.com:s=zohoarc:i=1");
	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";
	dmarc=none
X-Migadu-Scanner: scn1.migadu.com
X-Migadu-Spam-Score: -6.75
X-TUID: aR5EKZQCX7YQ


 ---- On Sat, 31 Dec 2022 07:56:10 -0500  Ihor Radchenko  wrote ---=20
 > As for being a macro, there will be not much gain - the convention is
 > mostly designed for things like `cl-defun' aimed to be used in the code.
 > `org-babel-shell-initialize' is only used by `org-babel-shell-names'.

I'm not sure what you mean by "to be used in code".  Do you mean called wit=
hin the global scope?

 > I do not have objections if it were a macro though. (But I do not see
 > how it would help debugging).
 >
 > > Because `org-babel-shell-initialize' is a function factory, you can't =
easily examine or modify their definitions.  `C-h f org-babel-execute:sh' j=
umps to the top of lisp/ob-shell.el.  Changing the definition requires reev=
aluating the definition for all the execute functions (or first changing `o=
rg-babel-shell-names').
 >=20
 > This is indeed a downside. Any better ideas?
 > ob-core dictates that we must have org-babel-execute:lang functions to
 > make things work.

My apologies, I feel there are too many separate issues I've brought up.  S=
ince I've already brought them up, let me try to be more clear about them. =
=20

I observe four issues with the current form of `org-babel-shell-initialize'=
:

1. The source is not explicit for a given `org-babel-execute:some-shell', m=
aking it difficult to debug
2. Source navigation gets confused when looking up help for a generated fun=
ction.  For example, `C-h f org-babel-execute:sh' goes to the top of ob-she=
ll.el
3. It does not adhere to the coding convention
4. Except for using the customize interface, changing `org-babel-shell-name=
s' requires reevaluating the function generator, currently `org-babel-shell=
-initialize'

Here's my perspective on each point.

1. The source is not explicit for a given `org-babel-execute:some-shell', m=
aking it difficult to debug

The benefit of using a macro is clarity of the defined functions.  Here's t=
he core `org-babel-shell-initialize' functionality as a macro.  Note that i=
t doesn't loop through `org-babel-shell-names'.

(defmacro define-babel-shell-1 (shell-name)
  (declare (indent nil) (debug t))
  (let ((function-name (intern (concat "my-org-babel-execute:" shell-name))=
))
    `(progn
       (defun ,function-name (body params)
         ,(format "Execute a block of %s commands with Babel." shell-name)
         (let ((shell-file-name ,shell-name)
               (org-babel-prompt-command
                (or (alist-get ,shell-name org-babel-shell-set-prompt-comma=
nds)
                    (alist-get t org-babel-shell-set-prompt-commands))))
           (org-babel-execute:shell body params)))
       (defalias
         ',(intern (concat "org-babel-variable-assignments:" shell-name))
         'org-babel-variable-assignments:shell
         ,(format "Return list of %s statements assigning to the block's va=
riables." shell-name))
       (defvar ,(intern (concat "org-babel-default-header-args:" shell-name=
)) nil))))

You can expand to see the definitions:

(pp-macroexpand-expression '(define-babel-shell-1 "csh"))

Is there a way to see the definition of`org-babel-execute:csh' using the cu=
rrent `org-babel-shell-initialize', that is, when generated by a function?

Looking at the expansion, I see what appears to be an error:

(alist-get "csh" org-babel-shell-set-prompt-commands)

`org-babel-shell-set-prompt-commands' is an alist keyed by string shell nam=
es and whose values are shell commands to set the prompt.  `alist-get' uses=
 `eq' by default which always returns nil for string comparisons.  That is,=
 (alist-get "csh" org-babel-shell-set-prompt-commands) returns nil, not bec=
ause the corresponding alist value is nil.  Rather, because the (eq "csh" "=
csh") comparison evaluates as nil.  The TESTFN probably should be `equal' o=
r `string=3D':

(alist-get "csh" org-babel-shell-set-prompt-commands nil nil 'equal)

Then, it will return the prompt associated with "csh".

All this is visible from the function version of `org-babel-shell-initializ=
e', of course.  However, it requires mentally tracking that ,name resolves =
to a string.  Using a macro along with `pp-macroexpand-expression' makes th=
e defined function explicit.  The forms generated by the macro expansion ar=
e readily available to eval whereas the function version makes the definiti=
ons inaccessible (AFAICT). =20

2. Source navigation gets confused when looking up help for a generated fun=
ction.  For example, `C-h f org-babel-execute:sh' goes to the top of ob-she=
ll.el

Generating the execute functions with a macro also has this problem.  I'm n=
ot sure there's a way around it other than defining each function with `def=
un'.  Doing that would be a bad idea because of the massive code duplicatio=
n/maintenance it would create.

3. It does not adhere to the coding convention.

I'll requote the convention here for convenience.

> (elisp) Coding Conventions says,
>
> "=E2=80=A2 Constructs that define a function or variable should be macros=
, not
> functions, and their names should start with =E2=80=98define-=E2=80=99. T=
he 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."

It's not clear to me why the convention exists, beyond consistency (a valid=
 reason).  I suspected it was to make the code clearer (points 1) and to "h=
elp various tools find the definition automatically" (point 2). =20

I'm biased by my experience into thinking a macro actually addresses point =
1.  I could be wrong about it, though. It could just have been luck and per=
sonal preference, and I may be overlooking some hidden complexity, a common=
 problem with macros.  Is there anything you see that I might be overlookin=
g?

AFAICT, a macro doesn't help with finding the definition of the generated f=
unction.  Any idea what tools it's talking about?

Also, the way I defined `define-babel-shell-1' doesn't fit the convention. =
 Something like this would:

(defmacro define-babel-execute-shell-2 (name)
  "Define functions and variables needed by Org Babel to execute a shell.

NAME is a symbol of the form `org-babel-execute:my-shell'."
  (declare (indent nil) (debug t))
  (let ((shell-program (cadr (split-string (symbol-name name) ":"))))
    `(progn
       (defun ,name (body params)
         ,(format "Execute a block of %s commands with Babel." shell-progra=
m)
         (let ((shell-file-name ,shell-program)
               (org-babel-prompt-command
                (or (alist-get ,shell-program org-babel-shell-set-prompt-co=
mmands)
                    (alist-get t org-babel-shell-set-prompt-commands))))
           (org-babel-execute:shell body params)))
       (defalias
         ',(intern (concat "org-babel-variable-assignments:" shell-program)=
)
         'org-babel-variable-assignments:shell
         ,(format "Return list of %s statements assigning to the block's va=
riables." shell-program))
       (defvar ,(intern (concat "org-babel-default-header-args:" shell-prog=
ram)) nil))))

AFAICT, adhering strictly to the convention would make things needlessly co=
mplicated.  The execute function's symbols would need to be interned before=
hand, creating an extra step between the `org-babel-shell-names' and the ex=
ecute functions, only for them to be converted and parsed out in the macro:

(intern "org-babel-execute:test")
(symbolp 'org-babel-execute:test)
(pp-macroexpand-expression '(define-babel-execute-shell-2 org-babel-execute=
:test))

4. Except for using the customize interface, changing `org-babel-shell-name=
s' requires reevaluating the function generator (`org-babel-shell-initializ=
e' or some variant of `define-babel-shell-1' )

A macro would not solve the need to re-evaluate the function generation whe=
n a change is made to `org-babel-shell-names'.  Either way, we need to loop=
/map over the list of shells.

I'm curious your thoughts about removing the `:set' function from `org-babe=
l-shell-names' and using `add-variable-watcher' instead.  In my exploration=
s, the watcher gets called when using customize, as well as when a new shel=
l is added to `org-babel-shell-names' using `add-to-list'.