From: Kyle Meyer <firstname.lastname@example.org>
To: Matt Huszagh <email@example.com>
Cc: Emacs-Orgmode <firstname.lastname@example.org>
Subject: Re: [PATCH] Omit file description when :file-desc has nil value
Date: Thu, 24 Sep 2020 01:23:23 -0400 [thread overview]
Message-ID: <email@example.com> (raw)
Matt Huszagh writes:
>> Kyle Meyer <firstname.lastname@example.org> 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 . It may not be intuitive, but
it's longstanding and I don't have a sense for how much it's relied on.
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
> 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
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 7300f239e..4483585a1 100644
@@ -646,6 +646,13 @@ (defun org-babel--expand-body (info)
(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)))
(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
- (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))
(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)))
+ (org-babel--file-desc (nth 2 info) 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
@@ -1084,7 +1084,16 @@ (ert-deftest test-ob/file-desc-header-argument ()
(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 
+ (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.
next prev parent reply other threads:[~2020-09-24 5:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-05 19:19 [PATCH] Omit file description when :file-desc has nil value Huszaghmatt
2020-09-06 4:19 ` Kyle Meyer
2020-09-09 19:50 ` Matt Huszagh
2020-09-15 17:09 ` Matt Huszagh
2020-09-24 5:23 ` Kyle Meyer [this message]
2020-09-24 6:16 ` Matt Huszagh
2020-09-24 23:07 ` Kyle Meyer
2020-09-29 21:33 ` Matt Huszagh
2020-10-03 6:08 ` Kyle Meyer
2020-10-06 13:17 ` Matt Huszagh
2020-10-07 3:19 ` Kyle Meyer
-- strict thread matches above, loose matches on Subject: below --
2020-09-03 6:19 Matt Huszagh
2020-09-03 6:53 ` Matt Huszagh
2020-09-04 5:21 ` Kyle Meyer
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).