From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id OLYoIBEbi2AHcwEAgWs5BA (envelope-from ) for ; Thu, 29 Apr 2021 22:46:09 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id iKjRGxEbi2BldAAAbx9fmQ (envelope-from ) for ; Thu, 29 Apr 2021 20:46:09 +0000 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 B266DB28B for ; Thu, 29 Apr 2021 22:46:08 +0200 (CEST) Received: from localhost ([::1]:36272 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lcDXz-00082L-Db for larch@yhetil.org; Thu, 29 Apr 2021 16:46:07 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:50898) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lcDXG-00082C-HE for emacs-orgmode@gnu.org; Thu, 29 Apr 2021 16:45:22 -0400 Received: from wout1-smtp.messagingengine.com ([64.147.123.24]:37119) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lcDX9-0002Lh-CP for emacs-orgmode@gnu.org; Thu, 29 Apr 2021 16:45:22 -0400 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.west.internal (Postfix) with ESMTP id 2C226F43 for ; Thu, 29 Apr 2021 16:45:12 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 29 Apr 2021 16:45:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-type:date:from:message-id :mime-version:subject:to:x-me-proxy:x-me-proxy:x-me-sender :x-me-sender:x-sasl-enc; s=fm2; bh=CxsXjCWyS5E1vjUN/kZpM8vniFokv ZcTGIAl2WlRhGs=; b=mBtCSBLh79+BNXbJiqYtS1c8D6y07K3OvofMaorwl6/mp 5c5h/0++libRPn+R/ssrBuj7ifrpoVkaa8vFrBJapli8kwTAb1MANZqi+qBD4l2H PwQ8UkvPXi0KZziHykk3doTehmCsCYJ4SE8N3MwpqvCDHk+8TdmosfFsTG7KTa4r VI9wUqDytLSte88SHkjkGSI96iFBkagmXR4/HKOX/NWj0bwwZH5vSWKZKY5HEM1Y 2KSMus+DviNxeG/OCGDNFLLEYLgNdWxYAuqWFXlsCb3so46rTwlBiVdkzS/NM1FE C2stLvfQ/rM2Qvh+ijYc0ZH1JVFIMVDXvKH5W+VLw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvddvgedgudehhecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhephffvuffkffgfgggtsehmtderre dtfeejnecuhfhrohhmpefpihgtkhcuufgrvhgrghgvuceonhhitghksehnihgtkhhsrghv rghgvgdrtggrqeenucggtffrrghtthgvrhhnpeejudduveevheduleffleehffeuteeige duffehjeelfedvgeejleevieffgfeijeenucfkphepudejgedrleefrdduieehrddufeen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehnihgtkh esnhhitghkshgrvhgrghgvrdgtrg X-ME-Proxy: Received: from [192.168.0.67] (bras-base-aylmpq0104w-grc-46-174-93-165-13.dsl.bell.ca [174.93.165.13]) by mail.messagingengine.com (Postfix) with ESMTPA for ; Thu, 29 Apr 2021 16:45:11 -0400 (EDT) From: Nick Savage To: emacs-orgmode@gnu.org Subject: [PATCH] ob-eval.el: Refactoring `org-babel-eval' to improve readability Message-ID: <87702cb1-0707-04c0-3a6d-138092787db9@nicksavage.ca> Date: Thu, 29 Apr 2021 16:45:10 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------EB057A5283E02681B91EC23C" Content-Language: en-US Received-SPF: none client-ip=64.147.123.24; envelope-from=nick@nicksavage.ca; helo=wout1-smtp.messagingengine.com X-Spam_score_int: -25 X-Spam_score: -2.6 X-Spam_bar: -- X-Spam_report: (-2.6 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 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" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1619729168; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=CxsXjCWyS5E1vjUN/kZpM8vniFokvZcTGIAl2WlRhGs=; b=TJ//IDESJB/DmpUSORR6WSv9TtVW0D9CeH1QI276qzQSPnni1O4+URIXzuC0aa4agWxxKf kSA3IlrK/hZqh7y1j1Or/fYVZVFswWGxE2VHxNkdlUaLn7R6u8XSj7bUvug6StduiFea9+ QU51lRKMb3b9RQeRDOJ5kyiBAcxU+X///9rzyu5R10QayCWRlSGmA57/D8Qf7XWl/oYZGP axOKaAbEqD5eNVZ6OWvf0e6J/RDWmv4wfO4u7tCpqv9CQlpBdLatxIsCtdi3q5YZfDz283 +LkP/DZI0zduCyVeb/U6OVbDnVtukiYbnlHN4NvmK3YLyvVq1Ldro6Pv5NvWJQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1619729168; a=rsa-sha256; cv=none; b=GQ/4/aTShB46fjpXUb4jSExMLO0roW3ZVenTz9XP4xm40sGyT0cwXH9dGUbHCeXwc5qHTp cNRW9gVYLF69R4qMEcG7TgjObxeqLgBQXtUVRuqN1KMcBMDJfcsm90mhZvGH7kgf/DEpew wn73FS0lVnVutuDPgEITjUni/7bckPTSnZmKk0GZAk2JUhqGzxMl0DK/V2LZtnxaJ4v/WK MgF5150cSQXUew14nU0sSBIiSVwr7F7++lrQiaEkieLfCzFo8dqRen059Rquvmom+pwoqS HNthGjRymhrXJBw06M2/oOdvGs4QWziuvfy4wwHEBR105FA6jFZyyDLKCic8GQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=messagingengine.com header.s=fm2 header.b=mBtCSBLh; spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Spam-Score: -2.66 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=messagingengine.com header.s=fm2 header.b=mBtCSBLh; dmarc=none; spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Queue-Id: B266DB28B X-Spam-Score: -2.66 X-Migadu-Scanner: scn0.migadu.com X-TUID: RypysUzAIjkf This is a multi-part message in MIME format. --------------EB057A5283E02681B91EC23C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hello everyone, I have attached a patch to refactor `org-babel-eval' and `org-babel--shell-command-on-region' to improve readability and to make local variables more consistent between functions. This also removes two parameters from `org-babel--shell-command-on-region', START and END. The function was created as a simplified `shell-command-on-region' that does only part of what it does. Those two parameters were included. As far as I can tell, `org-babel--shell-command-on-region' is only ever called by `org-babel-eval', and there is never any uncertainty over the contents of START and END as they are only ever (point-min) and (point-max). Given how specialized this function is, I believe it highly unlikely this is called anywhere else. I searched github too and didn't find anywhere where where code is calling this so I felt it was safe. I'm happy to change it back if necessary. Please let me know what you think. Thanks, Nick --------------EB057A5283E02681B91EC23C Content-Type: text/x-patch; charset=UTF-8; name="0001-ob-eval.el-Refactoring-org-babel-eval-to-improve-rea.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-ob-eval.el-Refactoring-org-babel-eval-to-improve-rea.pa"; filename*1="tch" >From ac0cdc66374f1c99aadd5f07e0c6d65eaaa158cc Mon Sep 17 00:00:00 2001 From: Nicholas Savage Date: Wed, 28 Apr 2021 06:15:47 -0400 Subject: [PATCH] ob-eval.el: Refactoring `org-babel-eval' to improve readability * ob-eval.el (org-babel-eval): Improve documentation and rename local variables to be consistent with `org-babel--shell-command-on-region' * (org-babel--shell-command-on-region): Remove START and END as parameters. * (org-babel--shell-command-on-region): Refactored out parts of function to `org-babel--write-temp-buffer-input-file' and `org-babel--get-shell-file-name'. This removes two parameters from `org-babel--shell-command-on-region'. It appears that START and END were parameters only because shell-command-on-region has them. This function is only called by org-babel-eval so it looks safe to remove those parameters, since they are always (point-min) and (point-max) and are never changed. Given the way the function works and that it is, it is unlikely that any user code relies on it. --- lisp/ob-eval.el | 87 ++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 37 deletions(-) diff --git a/lisp/ob-eval.el b/lisp/ob-eval.el index b0fca7bd9..cfde4f1ca 100644 --- a/lisp/ob-eval.el +++ b/lisp/ob-eval.el @@ -41,20 +41,22 @@ (display-buffer buf)) (message "Babel evaluation exited with code %S" exit-code)) -(defun org-babel-eval (cmd body) - "Run CMD on BODY. -If CMD succeeds then return its results, otherwise display -STDERR with `org-babel-eval-error-notify'." - (let ((err-buff (get-buffer-create " *Org-Babel Error*")) exit-code) - (with-current-buffer err-buff (erase-buffer)) +(defun org-babel-eval (command query) + "Run COMMAND on QUERY. +Writes QUERY into a temp-buffer that is processed with +org-babel--shell-command-on-region. If COMMAND succeeds then return +its results, otherwise display STDERR with +`org-babel-eval-error-notify'." + (let ((error-buffer (get-buffer-create " *Org-Babel Error*")) exit-code) + (with-current-buffer error-buffer (erase-buffer)) (with-temp-buffer - (insert body) + (insert query) (setq exit-code (org-babel--shell-command-on-region - (point-min) (point-max) cmd err-buff)) + command error-buffer)) (if (or (not (numberp exit-code)) (> exit-code 0)) (progn - (with-current-buffer err-buff + (with-current-buffer error-buffer (org-babel-eval-error-notify exit-code (buffer-string))) (save-excursion (when (get-buffer org-babel-error-buffer-name) @@ -71,26 +73,19 @@ STDERR with `org-babel-eval-error-notify'." (with-temp-buffer (insert-file-contents file) (buffer-string))) -(defun org-babel--shell-command-on-region (start end command error-buffer) +(defun org-babel--shell-command-on-region (command error-buffer) "Execute COMMAND in an inferior shell with region as input. +Stripped down version of `shell-command-on-region' for internal use in +Babel only. This lets us work around errors in the original function +in various versions of Emacs. This expects the query to be run to be +in the current temp buffer. This is written into +input-file. ERROR-BUFFER is the name of the file which +`org-babel-eval' has created to use for any error messages that are +returned." -Stripped down version of shell-command-on-region for internal use -in Babel only. This lets us work around errors in the original -function in various versions of Emacs. -" (let ((input-file (org-babel-temp-file "ob-input-")) (error-file (if error-buffer (org-babel-temp-file "ob-error-") nil)) - ;; Unfortunately, `executable-find' does not support file name - ;; handlers. Therefore, we could use it in the local case - ;; only. - (shell-file-name - (cond ((and (not (file-remote-p default-directory)) - (executable-find shell-file-name)) - shell-file-name) - ((file-executable-p - (concat (file-remote-p default-directory) shell-file-name)) - shell-file-name) - ("/bin/sh"))) + (shell-file-name (org-babel--get-shell-file-name)) exit-status) ;; There is an error in `process-file' when `error-file' exists. ;; This is fixed in Emacs trunk as of 2012-12-21; let's use this @@ -99,18 +94,13 @@ function in various versions of Emacs. (delete-file error-file)) ;; we always call this with 'replace, remove conditional ;; Replace specified region with output from command. - (let ((swap (< start end))) - (goto-char start) - (push-mark (point) 'nomsg) - (write-region start end input-file) - (delete-region start end) - (setq exit-status - (process-file shell-file-name input-file - (if error-file - (list t error-file) - t) - nil shell-command-switch command)) - (when swap (exchange-point-and-mark))) + (org-babel--write-temp-buffer-input-file input-file) + (setq exit-status + (process-file shell-file-name input-file + (if error-file + (list t error-file) + t) + nil shell-command-switch command)) (when (and input-file (file-exists-p input-file) ;; bind org-babel--debug-input around the call to keep @@ -135,6 +125,16 @@ function in various versions of Emacs. (delete-file error-file)) exit-status)) +(defun org-babel--write-temp-buffer-input-file (input-file) + "Write the contents of the current temp buffer into INPUT-FILE." + (let ((start (point-min)) + (end (point-max))) + (goto-char start) + (push-mark (point) 'nomsg) + (write-region start end input-file) + (delete-region start end) + (exchange-point-and-mark))) + (defun org-babel-eval-wipe-error-buffer () "Delete the contents of the Org code block error buffer. This buffer is named by `org-babel-error-buffer-name'." @@ -142,6 +142,19 @@ This buffer is named by `org-babel-error-buffer-name'." (with-current-buffer org-babel-error-buffer-name (delete-region (point-min) (point-max))))) +(defun org-babel--get-shell-file-name () + "Return system `shell-file-name', defaulting to /bin/sh. +Unfortunately, `executable-find' does not support file name +handlers. Therefore, we could use it in the local case only." + ;; FIXME: This is generic enough that it should probably be in emacs, not org-mode + (cond ((and (not (file-remote-p default-directory)) + (executable-find shell-file-name)) + shell-file-name) + ((file-executable-p + (concat (file-remote-p default-directory) shell-file-name)) + shell-file-name) + ("/bin/sh"))) + (provide 'ob-eval) ;;; ob-eval.el ends here -- 2.20.1 --------------EB057A5283E02681B91EC23C--