From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.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 cJMHLKl4rmNs0wAAbAwnHQ (envelope-from ) for ; Fri, 30 Dec 2022 06:35:37 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id aCkLLKl4rmPpqgAA9RJhRA (envelope-from ) for ; Fri, 30 Dec 2022 06:35:37 +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 3B0DBEA38 for ; Fri, 30 Dec 2022 06:35:37 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pB82c-0008Nq-VU; Fri, 30 Dec 2022 00:34:50 -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 1pB82b-0008NX-5o for emacs-orgmode@gnu.org; Fri, 30 Dec 2022 00:34:49 -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 1pB82Z-0005f4-AL; Fri, 30 Dec 2022 00:34:48 -0500 ARC-Seal: i=1; a=rsa-sha256; t=1672378479; cv=none; d=zohomail.com; s=zohoarc; b=ZuKjBshlLHmV1PCMZ/Fr0rjZb1hhw8QHQJnErhrlY8Mus0Pd6UcO0nLIjVkQR5GMMuj6rA8UskMhw7Vs0UKhPKvvaasRsrDB/NC+3a4BGfwmmA//1ycrl+lI/c4Zoap0UsxlsvVNGPJZ2Mch/wSXgKEDkU+43haafvhXBDZrdBc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1672378479; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=g7MeKUX5X8Nri74p+Lajish5J97pg1XLPP11JUhsIhY=; b=Uosg6ECeU6Gsl4/gQB3ARJFbPewlFIG4nlCFV5DTVeRAeA8iUxC5nPru0PTDr9rDeq4dfSt7wvd60rFSzd618OZkzcgmSgm0it39px6O1+l3DzL25HB0z2o4xo+NfQ67zu86lNAW1KLRxbN223i5yBXkqkNSADCj1TClpCtcImg= 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=1672378479; 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=g7MeKUX5X8Nri74p+Lajish5J97pg1XLPP11JUhsIhY=; b=dvPmgooS0YFuwmCxX6Ny2RF6mpBVE3KAQNx+B+gf1LfwqYf0ThwWKw96nhmk+0TF rxruhVAMZibqkjL12m0JIPB6vArLNnQ2McYE+WZUaItlbyy4ooHaU9VWcV7wHENm53I 3rpK4TTfV0aIBg3BGbcFPYnI4fHE466srRNBFN6M= Received: from mail.zoho.com by mx.zohomail.com with SMTP id 1672378478496962.3653968703468; Thu, 29 Dec 2022 21:34:38 -0800 (PST) Date: Fri, 30 Dec 2022 00:34:38 -0500 From: Matt To: "Ihor Radchenko" Cc: "bastien" , "emacs-orgmode" Message-ID: <18561866f8c.f22a62b3866025.8634474263936272339@excalamus.com> In-Reply-To: <87edsiv54k.fsf@localhost> References: <878rj7s0ti.fsf@localhost> <1853354beb4.fce54d8d902653.6359367327256505471@excalamus.com> <18555580d75.d7b471f9370716.1497263973038999899@excalamus.com> <87edsiv54k.fsf@localhost> Subject: 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-Seal: i=2; s=key1; d=yhetil.org; t=1672378537; a=rsa-sha256; cv=pass; b=G963jvihRI8L2HD4eK2onC++6momhygMgKoOHQlh9nmLP3fxRZYWXrQDl8IkWoAX6HF0ai /UVQNPY/4CPBZf3vYW9642RxrMci10HHL5vvn08NejgdCFimWLCngMNunAwPUmNBronO+2 Ah4jJW1zoXQH8Wjm/uRrHYckSc6T7l0mDll2ZHzusL+yrMy3HIzMsddfS+urLju3/px3do d7HjkxH9dLOTaLdLE64dXroHIGjf6SFr2sB7fbIFatjzFbs9z0xyHmtaTG/mOeBLjBVuHO 5mBiLqvo7lRuoOZ/B1cmVEpL7PQWh9rXI2bOhMP0fNcwIIAuCIizuX1Oak7u6w== ARC-Authentication-Results: i=2; aspmx1.migadu.com; dkim=pass header.d=excalamus.com header.s=zmail header.b=dvPmgooS; 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=pass ("zohomail.com:s=zohoarc:i=1") ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1672378537; 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=g7MeKUX5X8Nri74p+Lajish5J97pg1XLPP11JUhsIhY=; b=feQl0cdJTJNiZpFyiq10uIRr8JnCA5w+5WLkHfaLu2d/MLQfy5AYTWbDPllrhqlStl30Cs YqXTtAp8MK/YxXSB1mjamAB9TLnTBp/LMZPGowFVaRpYm94fF1yCU+2nheJv2tfa+MydtS rKQ+cRHGEBc6vkPVhh4V96j8mv2YTuhQTqJQe49W4LnSAWdQgy6sfBW1krQGqWvU54PPhJ k+dqJA7ldfdMPer0yp76yo8NKremXYOWTdyRoqvEUFL4SCtT+18bp8/0QCiwKf67ut4mAZ b4m2/8hY1Zk+ZiWGu9AlSW2ZHjnVe/DIrfsf68gym0mjFQl+ocVNnZA8Mq5CyQ== X-Spam-Score: -10.01 X-Migadu-Queue-Id: 3B0DBEA38 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=excalamus.com header.s=zmail header.b=dvPmgooS; 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=pass ("zohomail.com:s=zohoarc:i=1") X-Migadu-Scanner: scn0.migadu.com X-Migadu-Spam-Score: -10.01 X-TUID: eVdxA39OW56r ---- On Thu, 29 Dec 2022 06:08:59 -0500 Ihor Radchenko wrote ---=20 =20 > Does it mean that you are willing to maintain lisp/ob-shell.el? > We usually give write access to the maintainers and regular > contributors. AFAIR, you previously contributed to WORG but not to Org > core. You're correct, I've not contributed to core. I would love to maintain li= sp/ob-shell.el. I'm expecting life changes in the coming months and can't = anticipate how that will affect my time. Would it be a problem if I need t= o step down as maintainer for a period? > > From: Matt Trzcinski matt@excalamus.com> > >=20 > > [fn:1] Session name is a string and not a variable (such as > > `session-name'). This is because `org-babel-execute:sh' produces a > > type error when the session name is defined with a variable. The > > source of the error appears to be the `params' symbol in > > `org-babel-execute:sh'. `params' does not evaluate as a variable as > > expected=E2=80=93it evaluates as a symbol. However, `org-babel-execut= e:sh' is > > defined within a function factory which makes it difficult to debug. > > Hard code the test name for now and refactor it later once > > `org-babel-execute:sh` is refactored. >=20 > There is nothing wrong here. `org-babel-execute-src-block' takes care > about parameter processing making sure that :session value is always a > string. =20 Originally the test used "yes" for a comparison string and the shell it ran= in. I changed "yes" to be the test name because when it was "yes", I did= n't know what test had produced it. However, the problem with the test nam= e being hard coded as a string is that if the function name changes, the te= st string may get out of sync. More on this below. > > +;; TODO refactor into macro. Currently violates (elisp) Coding > > +;; Conventions and is hard to debug. > > (defun org-babel-shell-initialize () > > "Define execution functions associated to shell names. >=20 > Could you please elaborate? Which particular convention does it violate? > What is hard to debug? (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= . 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." The `org-babel-shell-initialize' function defines *all* the `org-babel-exec= ute:XXX' functions given by `org-babel-shell-names' (sh, bash, zsh, etc.). Because `org-babel-shell-initialize' is a function factory, you can't easil= y examine or modify their definitions. `C-h f org-babel-execute:sh' jumps = to the top of lisp/ob-shell.el. Changing the definition requires reevaluat= ing the definition for all the execute functions (or first changing `org-ba= bel-shell-names'). This was a problem for me when I wanted to make the session name string for= `test-ob-shell/session' the test name (mentioned above). In the test, whe= n I replaced the session name string with a variable containing the string,= `org-babel-execute:sh' failed with a type error. I couldn't get the varia= ble to evaluate (with backquote and comma or otherwise). Without an explic= it function definition or a macro to expand, I found it hard to debug/exper= iment with (and so left the test name as a hard coded string). > > (ert-deftest test-ob-shell/dont-insert-spaces-on-expanded-bodies () > > "Expanded shell bodies should not start with a blank line unless > > -the body of the tangled block does." > > +the strings of the tangled block does." >=20 > What does "strings of the tangled block" refer to? The previous variant > is a lot more clear for me. I believe this is a query-replace error. Good catch! > > +(ert-deftest test-ob-shell/generic-uses-no-arrays () > > + "Test generic serialization of array into a single string." > > + (org-test-with-temp-text > > + (test-ob-shell-multiline-string > > + "#+NAME: sample_array" > > + "| one |" > > + "| two |" > > + "| three |" >=20 > Why do you need `test-ob-shell-multiline-string' here? > Can simply type-in the string directly, as the rest of tests do. I probably don't need it and am happy to remove it. An older version of = the function was more complex and made sense as a separate function (or so = I thought). My aim was to make the test strings easy to read so that it's = clearer what's being tested (i.e. not write multi-line strings on a single = line). I could use concat and add "\n" to the end of each line. Or, simpl= y write out the string-join. Maybe there's another way to write multi-line= strings that I'm not aware of in Elisp, maybe something like Python's trip= le-quote? > > +(require 'org-test (expand-file-name "../org-test.el")) >=20 > I am unsure here. What will happen if you run this file from > default-directory not the same with file location? I'm unsure if this is the best approach. AFAICT, it works. For example, (let ((default-directory "/home/ahab/Documents/")) (load "/home/ahab/Projects/org-mode/testing/lisp/test-ob-shell.el")) The problem I'm trying to solve is that the test file relies on `org-test' = yet doesn't declare it. Instead, you need to know to load it ahead of time= (or run into the error when trying to load the tests). > Also, the repetitive patches changing names + killing error buffer for > individual tests can be merged into a single patch. Thank you for taking the time to review everything. I'll get a new patch s= et created that includes updates based on all your feedback. =20