Kyle Meyer <k...@kyleam.com> writes: > 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.
Yep, I agree. I think my phrasing could have just been better. I meant to include the breakage as a factor against. > 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. I like this solution better than mine. I guess it's still a bit of a hack, but it doesn't seem to be one that could break a use case, whereas the empty string could conceivably be intended, though I'm still not sure why. > 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? Yes I think you're right. I was referring to my solution of an empty string, which I didn't see a personal use for, but felt it might be a valid use case for someone else. I really can't think of any reason the empty vector would otherwise be valid. > 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. This patch looks good. I've tested it and it works well for me. Thanks for coming up with a good solution! I think the one thing still missing is some documentation in the info manual. Something along the lines of The ‘file-desc’ header argument defines the description (see *note Link Format::) for the link. If ‘file-desc’ has no value, Org uses the generated file name for both the “link” and “description” parts of the link. If you want to omit the description (i.e., [[link]]), you can either omit the ‘file-desc’ header argument or provide it with an empty vector (i.e., :file-desc []). Feel free to add this (or something else) to your patch. Or, if you'd prefer that I created a patch for it, let me know. Matt