From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ) for ; 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 ) for ; 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 ; 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 ) 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 ) 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 ) 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= 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 To: "Ihor Radchenko" Cc: "emacs-orgmode" Message-ID: <18570c77eb9.119e6407e40169.7047396916380100893@excalamus.com> In-Reply-To: <87r0wfafzu.fsf@localhost> References: <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." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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'.