Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer
Joseph Turner writes: > I've added a couple of (message ...) blocks to log the value of > do-export and params. It appears that this combination is producing a > params value which is equivalent to the value of params produced by the > current org-babel-execute:plantuml function. However, when the above > function is executed on a block like: > > #+begin_src plantuml :file "this.png" > Bob->Alice : Hello1! > #+end_src > > I get "Code block produced no output." > > I suspect that setting the scoped params variable has no effect on the > execution of the function, since I can set params to '((:results . > "none")), and I'll still get a printed result if > org-babel-default-header-args:plantuml is set to the above value. This is because you did not really modify the params value. Inserting results happens outside the org-babel-execute:plantuml. You can check out org-babel-execute-src-block function. (params (if do-export (map-merge 'list params '((:results . "file") (:result-params "replace" "file"))) params) This snippet only binds the value locally, restoring it after exiting the let-binding. You may need to modify the params destructively using setf and assq. > Is it safe to modify the value of org-babel-default-header-args:plantuml > from within the function? Would that even work? This is not only unsafe, but will also have no effect on the current execution. The passed params argument is a result of parsing global header args, default header args for specific language, file-local header args, subtree-local header args, and src block-specific header args combined. Strictly speaking, it is normal for org-babel to require the user to specify the output format explicitly via :results output or :results file or any other available setting. Having a :file argument must be accompanied by :results file. However, the ob-plantuml default value makes this generic org-babel requirement not necessary, which was a useful feature as long as only the :file output was supported. The most self-consistent way to introduce non-file output would be changing the default header args for plantuml to nil - it would make things consistent with all other generic backends. Unfortunately, such change is not an option because it will break the existing Org files for users accustomed to :file header arg implying :results file in their existing src blocks. Probably the most consistent way to approach the new output format would be throwing an error if :file is not provided, but :results is set to file. I'd say that it will be much more consistent with other babel backends, albeit less "smart". Best, Ihor
Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer
Ihor Radchenko writes: > You also need to change :result-params and :result-type. > See `org-babel-execute-src-block'. Here's what I've got so far: ``` (defvar org-babel-default-header-args:plantuml '((:exports . "results")) "Default arguments for evaluating a plantuml source block.") (defun org-babel-execute:plantuml (body params) "Execute a block of plantuml code with org-babel. This function is called by `org-babel-execute-src-block'." (let* ((do-export (cdr (assq :file params))) (params (if do-export (map-merge 'list params '((:results . "file") (:result-params "replace" "file"))) params)) (out-file (or do-export (org-babel-temp-file "plantuml-" ".txt"))) ; if ":file" is not provided, don't export, just print ASCII results (cmdline (cdr (assq :cmdline params))) (in-file (org-babel-temp-file "plantuml-")) (java (or (cdr (assq :java params)) "")) (executable (cond ((eq org-plantuml-exec-mode 'plantuml) org-plantuml-executable-path) (t "java"))) (executable-args (cond ((eq org-plantuml-exec-mode 'plantuml) org-plantuml-executable-args) ((string= "" org-plantuml-jar-path) (error "`org-plantuml-jar-path' is not set")) ((not (file-exists-p org-plantuml-jar-path)) (error "Could not find plantuml.jar at %s" org-plantuml-jar-path)) (t (list java "-jar" (shell-quote-argument (expand-file-name org-plantuml-jar-path)) (full-body (org-babel-plantuml-make-body body params)) (cmd (mapconcat #'identity (append (list executable) executable-args (pcase (file-name-extension out-file) ("png" '("-tpng")) ("svg" '("-tsvg")) ("eps" '("-teps")) ("pdf" '("-tpdf")) ("tex" '("-tlatex")) ("vdx" '("-tvdx")) ("xmi" '("-txmi")) ("scxml" '("-tscxml")) ("html" '("-thtml")) ("txt" '("-ttxt")) ("utxt" '("-utxt"))) (list "-p" cmdline "<" (org-babel-process-file-name in-file) ">" (org-babel-process-file-name out-file))) " "))) (with-temp-file in-file (insert full-body)) (message "\n%s\n" do-export) (message "\n%s\n" params) (message "%s" cmd) (org-babel-eval cmd "") (if (and (string= (file-name-extension out-file) "svg") org-babel-plantuml-svg-text-to-path) (org-babel-eval (format "inkscape %s -T -l %s" out-file out-file) "")) (unless do-export (with-temp-buffer (insert-file-contents out-file) (buffer-substring-no-properties (point-min) (point-max)) ``` I've added a couple of (message ...) blocks to log the value of do-export and params. It appears that this combination is producing a params value which is equivalent to the value of params produced by the current org-babel-execute:plantuml function. However, when the above function is executed on a block like: #+begin_src plantuml :file "this.png" Bob->Alice : Hello1! #+end_src I get "Code block produced no output." I suspect that setting the scoped params variable has no effect on the execution of the function, since I can set params to '((:results . "none")), and I'll still get a printed result if org-babel-default-header-args:plantuml is set to the above value. Is it safe to modify the value of org-babel-default-header-args:plantuml from within the function? Would that even work? Thank you for your patience in figuring this out with me :) Joseph
Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer
Joseph Turner writes: > Ihor Radchenko writes: >> You can examine :result-params property inside params plist. If that >> property does not explicitly mention different results Type (see 16.6 >> Results of Evaluation), ob-plantuml may set the type to "file" with >> plist-put. > > Perhaps I'm confused, but I think org-babel-default-header-args:plantuml > is actually an alist, right? Yes, you are right indeed. > I tried removing the (:results . "file") from > org-babel-default-header-args:plantuml, and then overwriting the params > argument inside the let* block like so: > > ``` > (let* ((do-export (cdr (assq :file params))) > (params (if do-export > (add-to-list 'params '(:results . "file"))) > (out-file ... > ``` > > Logging the params variable after the let* block reveals that :results > is set to "file", but I still get "Code block produced no output" when > I try to evaluate the plantuml org src block. > > Thoughts? You also need to change :result-params and :result-type. See `org-babel-execute-src-block'. Best, Ihor
Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer
Ihor Radchenko writes: > You can examine :result-params property inside params plist. If that > property does not explicitly mention different results Type (see 16.6 > Results of Evaluation), ob-plantuml may set the type to "file" with > plist-put. Perhaps I'm confused, but I think org-babel-default-header-args:plantuml is actually an alist, right? I tried removing the (:results . "file") from org-babel-default-header-args:plantuml, and then overwriting the params argument inside the let* block like so: ``` (let* ((do-export (cdr (assq :file params))) (params (if do-export (add-to-list 'params '(:results . "file"))) (out-file ... ``` Logging the params variable after the let* block reveals that :results is set to "file", but I still get "Code block produced no output" when I try to evaluate the plantuml org src block. Thoughts? Joseph
Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer
Joseph Turner writes: >> The solution will be simply removing the default :results setting. > > I think you're suggesting something like this: > > (defvar org-babel-default-header-args:plantuml > '((:exports . "results")) > "Default arguments for evaluating a plantuml source block.") > > With this change, if you *do* add a :file arg, like in the following > example, then no output will be produced: > > #+begin_src plantuml :file "this.png" > Bob->Alice : Hello1! > #+end_src > > #+RESULTS: > > which is also wrong. > What would the code look like if we wanted to change the > org-babel-default-header-args:plantuml variable inside the > org-babel-execute:plantuml function based on the value of the params > arg? Or perhaps you have a different solution? You can examine :result-params property inside params plist. If that property does not explicitly mention different results Type (see 16.6 Results of Evaluation), ob-plantuml may set the type to "file" with plist-put. Best, Ihor
Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer
Thank you for your feedback, Ihor! > Most importantly, the patch does not change the default value of > org-babel-default-header-args:plantuml. :results header arg is set to > "file" by default. Yes, I noticed this issue also. > The solution will be simply removing the default :results setting. I think you're suggesting something like this: (defvar org-babel-default-header-args:plantuml '((:exports . "results")) "Default arguments for evaluating a plantuml source block.") With this change, if you *do* add a :file arg, like in the following example, then no output will be produced: #+begin_src plantuml :file "this.png" Bob->Alice : Hello1! #+end_src #+RESULTS: which is also wrong. What would the code look like if we wanted to change the org-babel-default-header-args:plantuml variable inside the org-babel-execute:plantuml function based on the value of the params arg? Or perhaps you have a different solution? Once we straighten this issue out, I am happy to resubmit the updated patch with your suggested style changes. Warmly, Joseph
Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer
Joseph Turner writes: > Allow src block execution without ":file" header arg. When ":file" is > omitted, insert txt output in buffer below src block. > > TINYCHANGE Thanks for your contribution! This addition makes a lot of sense. Some comments: Most importantly, the patch does not change the default value of org-babel-default-header-args:plantuml. :results header arg is set to "file" by default. This yields something like the following: #+begin_src plantuml Bob->Alice : Hello1! #+end_src #+RESULTS: [[file: ,---. ,-. |Bob| |Alice| `-+-' `--+--' | Hello1! | |-->| ,-+-. ,--+--. |Bob| |Alice| `---' `-' ]] Note that the output is treated as a path to file: [[file: ... output string ...]] which is indeed wrong. The solution will be simply removing the default :results setting. Other comments are for code and documentation style. > Allow src block execution without ":file" header arg. When ":file" is > omitted, insert txt output in buffer below src block. Please use double space between sentences. See https://orgmode.org/worg/org-contribute.html > + > +*** =org-babel-execute:plantuml= can output ASCII graphs in the buffer > + > +Previously, executing PlantUML src blocks required a ":file" header > argument. Now, if that header argument is omitted, an ASCII graph will be > inserted below the src block. > + Likewise. Also, please fill the paragraph making sure that each line spans less than 80 columns. Same for the code and comments inside the code. > -nil)) ;; signal that output has already been written to file > +(unless do-export (with-temp-buffer > +(insert-file-contents out-file) > +(buffer-substring-no-properties (point-min) > (point-max)) > + We follow Elisp conventions regarding indentation and parenthesis placement as in D.1 Emacs Lisp Coding Conventions - do not leave parenthesis on the line of their own. Also, the way you typed unless leads to excessive indentation. You could instead do (unless do-export (with-temp-buffer ...)) Best, Ihor
[PATCH] lisp/ob-plantuml.el: Insert results in buffer
Allow src block execution without ":file" header arg. When ":file" is omitted, insert txt output in buffer below src block. TINYCHANGE --- etc/ORG-NEWS| 5 + lisp/ob-plantuml.el | 10 +++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 4cda357f1..1613fdab5 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -295,6 +295,11 @@ files that are exported to Texinfo. =org-at-heading-p= now returns t by default on headings inside folds. Passing optional argument will produce the old behaviour. + +*** =org-babel-execute:plantuml= can output ASCII graphs in the buffer + +Previously, executing PlantUML src blocks required a ":file" header argument. Now, if that header argument is omitted, an ASCII graph will be inserted below the src block. + ** Removed or renamed functions and variables *** =org-plantump-executable-args= is renamed and applies to jar as well diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el index ebbcdf166..f9b41c140 100644 --- a/lisp/ob-plantuml.el +++ b/lisp/ob-plantuml.el @@ -109,8 +109,9 @@ If BODY does not contain @startXXX ... @endXXX clauses, @startuml (defun org-babel-execute:plantuml (body params) "Execute a block of plantuml code with org-babel. This function is called by `org-babel-execute-src-block'." - (let* ((out-file (or (cdr (assq :file params)) - (error "PlantUML requires a \":file\" header argument"))) + (let* ((do-export (cdr (assq :file params))) + (out-file (or do-export + (org-babel-temp-file "plantuml-" ".txt"))) ; if ":file" is not provided, don't export, just print ASCII results (cmdline (cdr (assq :cmdline params))) (in-file (org-babel-temp-file "plantuml-")) (java (or (cdr (assq :java params)) "")) @@ -155,7 +156,10 @@ This function is called by `org-babel-execute-src-block'." (if (and (string= (file-name-extension out-file) "svg") org-babel-plantuml-svg-text-to-path) (org-babel-eval (format "inkscape %s -T -l %s" out-file out-file) "")) -nil)) ;; signal that output has already been written to file +(unless do-export (with-temp-buffer +(insert-file-contents out-file) +(buffer-substring-no-properties (point-min) (point-max)) + (defun org-babel-prep-session:plantuml (_session _params) "Return an error because plantuml does not support sessions." -- 2.37.0