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

2022-08-02 Thread Ihor Radchenko
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

2022-07-31 Thread Joseph Turner
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

2022-07-28 Thread Ihor Radchenko
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

2022-07-27 Thread Joseph Turner
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

2022-07-25 Thread Ihor Radchenko
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

2022-07-25 Thread 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

2022-07-25 Thread Ihor Radchenko
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

2022-07-21 Thread 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