[PATCH] lisp/ob-plantuml.el: Insert results in buffer

[PATCH] lisp/ob-plantuml.el: Insert results in buffer

[Joseph Turner]

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

Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer

[Ihor Radchenko]

Joseph Turner <joseph@breatheoutbreathe.in> 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

Re: [PATCH] lisp/ob-plantuml.el: Insert results in buffer

[Joseph Turner]

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

[Ihor Radchenko]

Joseph Turner <joseph@breatheoutbreathe.in> 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

[Joseph Turner]

Ihor Radchenko <yantar92@gmail.com> 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

[Ihor Radchenko]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Ihor Radchenko <yantar92@gmail.com> 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

[Joseph Turner]

Ihor Radchenko <yantar92@gmail.com> 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

[Ihor Radchenko]

Joseph Turner <joseph@breatheoutbreathe.in> 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