From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id YHtQFXAtbF+EAQAA0tVLHw (envelope-from ) for ; Thu, 24 Sep 2020 05:24:00 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id 8AElEXAtbF9cEgAAB5/wlQ (envelope-from ) for ; Thu, 24 Sep 2020 05:24:00 +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 389C894042A for ; Thu, 24 Sep 2020 05:23:59 +0000 (UTC) Received: from localhost ([::1]:46062 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kLJjZ-0006BH-Dr for larch@yhetil.org; Thu, 24 Sep 2020 01:23:57 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48804) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kLJjB-0006B6-Al for emacs-orgmode@gnu.org; Thu, 24 Sep 2020 01:23:33 -0400 Received: from pb-smtp20.pobox.com ([173.228.157.52]:52478) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kLJj8-0003Aw-LH for emacs-orgmode@gnu.org; Thu, 24 Sep 2020 01:23:32 -0400 Received: from pb-smtp20.pobox.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id C892EDC3E6; Thu, 24 Sep 2020 01:23:27 -0400 (EDT) (envelope-from kyle@kyleam.com) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:in-reply-to:references:date:message-id:mime-version :content-type; s=sasl; bh=U1eL0QPQ4SbvLlBghwOVuUz4Zjk=; b=LWZ8Xd 8BPsvlYVKBEQugMTJi4n2ks58FBhn4vl+Qq/F13Z0LRITtuAxQS4q/LU6x6xViiV febcwR82KIUf6MvXHzDYHH1qlhkQiBQrGEwV12AguYoyBeq4/esaip2jj9gCjG5A eqXzPoIb4e3cmFVc7LG4nf5vA5jmfT9G8Xi+M= Received: from pb-smtp20.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id C1DB9DC3E5; Thu, 24 Sep 2020 01:23:27 -0400 (EDT) (envelope-from kyle@kyleam.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=kyleam.com; h=from:to:cc:subject:in-reply-to:references:date:message-id:mime-version:content-type; s=mesmtp; bh=2gqx91sSkDP5Mpj9bhZmLM32tEI5I+htsHTZZMDsBuk=; b=Bvm14wsPqyqNYTzI79Jr8rasSaHuj82fB3giJpKG3NZtTKulyhz5rJ6xBolv7DQKqjl0dZNHGLHefyr6vuf7tq22ZBq5TFG3RabNak4FrXwkkFVgyGbUPw9WlS70EwZknO0mgaN1dIWsv3chfRmOr5DZlFdJop3OUz/lBaalaDA= Received: from localhost (unknown [45.33.91.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp20.pobox.com (Postfix) with ESMTPSA id 329A0DC3E4; Thu, 24 Sep 2020 01:23:25 -0400 (EDT) (envelope-from kyle@kyleam.com) From: Kyle Meyer To: Matt Huszagh Subject: Re: [PATCH] Omit file description when :file-desc has nil value In-Reply-To: <877dsv3r1w.fsf@gmail.com> References: <87r1rf35bb.fsf@kyleam.com> <87pn6uzq41.fsf@gmail.com> <877dsv3r1w.fsf@gmail.com> Date: Thu, 24 Sep 2020 01:23:23 -0400 Message-ID: <87a6xfvjck.fsf@kyleam.com> MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: 11FD0814-FE26-11EA-A5B7-F0EA2EB3C613-24757444!pb-smtp20.pobox.com Received-SPF: pass client-ip=173.228.157.52; envelope-from=kyle@kyleam.com; helo=pb-smtp20.pobox.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/24 01:23:28 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 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_LOW=-0.7, 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.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emacs-Orgmode Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=pobox.com header.s=sasl header.b=LWZ8Xd 8; dkim=pass header.d=kyleam.com header.s=mesmtp header.b=Bvm14wsP; 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-Spam-Score: -1.21 X-TUID: kraOQX4k+uNx Matt Huszagh writes: >> Kyle Meyer writes: >> >>> I also don't find the current behavior particularly intuitive. (I'm >>> also not really a babel user, so my opinion probably shouldn't count for >>> much.) If we were adding it today, I think what you describe would be >>> better, but, as you mention, breakage also now also weighs against >>> making a change here. >>> >>> In any case, I'd suggest raising the discussion on the list after the >>> 9.4 release. > > Hello, just following up on this since 9.4 has been released. Thoughts? No babel users have chimed in. My current opinion is that I'd prefer not to break the use case mentioned earlier in this discussion [1]. It may not be intuitive, but it's longstanding and I don't have a sense for how much it's relied on. [1] https://orgmode.org/list/87tuwef76g.fsf@kyleam.com/ https://orgmode.org/list/87limm4eo2.fsf@med.uni-goettingen.de/T/#u Quoting what you said earlier: > For one, I think that the current implementation is a bit > confusing. More importantly though, it makes it impossible to both > provide a default value for :file-desc and omit it in some cases. The > benefit (as mentioned in that thread) is that in those select cases, > the same argument would not need to be provided twice. I think the > cost of the current functionality outweighs the benefit. But it's not a direct comparison against that use case and the use case you want to support. The potential breakage of existing documents is a big factor to go against. > I guess there are other solutions we could explore, such as an empty > string (is an empty file descriptor ever a valid use case?) taking the > place of the current functionality, or fully eliminating the file > descriptor. However, this is starting to feel like a lot of hacks and > would be very confusing to new users. Unfortunately, such a kludge is how I'd suggest to move forward. Perhaps an empty string, or perhaps any value (e.g., ":file-desc []") that org-babel-read won't treat as a string or nil (the two cases that mean something right now). The rough patch below is an example of the latter. > Moreover, it really just pushes the problem down the road rather than > fixing it outright. I'm not sure I get this. What's next down the road in this scenario? With something like the above kludge, haven't we exhausted the cases for :file-desc? diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 7300f239e..4483585a1 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -646,6 +646,13 @@ (defun org-babel--expand-body (info) (replace-regexp-in-string (org-src-coderef-regexp coderef) "" expand nil nil 1)))) +(defun org-babel--file-desc (params result) + (pcase (assq :file-desc params) + (`nil nil) + (`(:file-desc) result) + (`(:file-desc . ,(and (pred stringp) val)) val) + (_ nil))) + ;;;###autoload (defun org-babel-execute-src-block (&optional arg info params) "Execute the current source code block. @@ -749,8 +756,7 @@ (defun org-babel-execute-src-block (&optional arg info params) (let ((*this* (if (not file) result (org-babel-result-to-file file - (let ((desc (assq :file-desc params))) - (and desc (or (cdr desc) result))))))) + (org-babel--file-desc params result))))) (setq result (org-babel-ref-resolve post)) (when file (setq result-params (remove "file" result-params)))))) @@ -2257,9 +2263,8 @@ (defun org-babel-insert-result (result &optional result-params info hash lang) (setq result (org-no-properties result)) (when (member "file" result-params) (setq result (org-babel-result-to-file - result (when (assq :file-desc (nth 2 info)) - (or (cdr (assq :file-desc (nth 2 info))) - result)))))) + result + (org-babel--file-desc (nth 2 info) result))))) ((listp result)) (t (setq result (format "%S" result)))) (if (and result-params (member "silent" result-params)) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index 648e9c115..e7a292de3 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -1084,7 +1084,16 @@ (ert-deftest test-ob/file-desc-header-argument () (org-babel-execute-src-block) (goto-char (point-min)) (should (search-forward "[[file:foo][bar]]" nil t)) - (should (search-forward "[[file:foo][foo]]" nil t)))) + (should (search-forward "[[file:foo][foo]]" nil t))) + (should (string-match-p + (regexp-quote "[[file:foo]]") + (org-test-with-temp-text " +#+begin_src emacs-lisp :results file :file-desc [] + \"foo\" +#+end_src" + (org-babel-next-src-block) + (org-babel-execute-src-block) + (buffer-substring-no-properties (point-min) (point-max)))))) (ert-deftest test-ob/result-file-link-type-header-argument () "Ensure that the result is a link to a file.