Re: [O] Add preamble support to ob-plantuml.el

[Nicolas Goaziou]

Thibault Marin  writes:

> Please find attached a patch updating ORG-NEWS.

Perfect. Applied. Thank you.

Regards,

Re: [O] Add preamble support to ob-plantuml.el

[Thibault Marin]

Hi,

Please find attached a patch updating ORG-NEWS.

Thanks,
thibault

>From 3d335b093d4e95b14cc71317d2aef024f1c64fd5 Mon Sep 17 00:00:00 2001
From: thibault 
Date: Sat, 10 Dec 2016 08:27:48 -0600
Subject: [PATCH] * etc/ORG-NEWS: Header arguments support for PlantUML source
 blocks

---
 etc/ORG-NEWS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index b6b3123..c115cf9 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -82,6 +82,11 @@ Where clue > 0
 ,#+END_SRC
 #+end_example
 
+ PlantUML: add support for header arguments
+
+[[http://plantuml.com/][Plantuml]] source blocks now support the [[http://orgmode.org/manual/prologue.html#prologue][~:prologue~]], [[http://orgmode.org/manual/epilogue.html#epilogue][~:epilogue~]] and
+[[http://orgmode.org/manual/var.html#var][~:var~]] header arguments.
+
 *** New variable : ~org-bibtex-headline-format-function~
 This allow to use a different title than entry title.
 *** Horizontal rules are no longer ignored in LaTeX table math mode
-- 
2.9.3

Re: [O] Add preamble support to ob-plantuml.el

[Nicolas Goaziou]

Hello,

Thibault Marin  writes:

> The attached patch removes the useless definition of
> `org-babel-plantuml-var-to-plantuml' (the regexp is moved to the
> `org-babel-variable-assignments:plantuml' function) but keeps the
> `org-babel-plantuml-make-body' function, useful for testing.

Fair enough. I applied your patch.

Would you mind providing an ORG-NEWS entry for the added feature?

> Thanks for the guidance.

Thank you for the work.

Regards,

-- 
Nicolas Goaziou

Re: [O] Add preamble support to ob-plantuml.el

[Thibault Marin]

Hi,

Nicolas Goaziou writes:
> Comments follow.
>
>> +(defun org-babel-plantuml-var-to-plantuml (var)
>> +  "Cleanup plantuml variable (remove quotes)."
>> + (replace-regexp-in-string "\"" "" var))
>
> Since this function is used only once in the code, I suggest to not
> implement it and use `replace-regexp-in-string' at the appropriate
> place.
I was trying to match what other ob-*s do.  If the table assignment idea
was to be implemented (for instance), using a separate function may be
cleaner.  But the function is indeed currently not needed, so I removed
it.

>> +(defun org-babel-variable-assignments:plantuml (params)
>> +  "Return a list of PlantUML statements assigning the block's variables."
>
> Could you document what is PARAMS?
I have added more complete docstrings, please let me know if changes are
required.

>> +(defun org-babel-plantuml-make-body (body params)
>> +  "Form PlantUML input string."
>
> Do you mean "Return PlantUML" input string? Also you need to specify
> what are body and params.
Tentatively done.

> Besides, the same applies to `org-babel-plantuml-var-to-plantuml' above.
> Is this function really needed, as it is a mere `format'.
I use this function in the test as well to compare the full text output
so it is convenient to have a separate function.  Alternatively I guess
I could directly test the call to `org-babel-expand-body:generic' but
that seems less interesting as a test (should I remove the test
altogether then?).

The attached patch removes the useless definition of
`org-babel-plantuml-var-to-plantuml' (the regexp is moved to the
`org-babel-variable-assignments:plantuml' function) but keeps the
`org-babel-plantuml-make-body' function, useful for testing.  If you
would like me to remove the `org-babel-plantuml-make-body' function as
well, I will do that (how would you like the test to look like in this
case?)

Thanks for the guidance.

thibault
>From 9e8addc14e628dc7c9c25d96d0cfd630ad15134e Mon Sep 17 00:00:00 2001
From: thibault 
Date: Fri, 9 Dec 2016 22:43:32 -0600
Subject: [PATCH] ob-plantuml.el: Add support for prologue and header variables

* lisp/ob-plantuml.el (org-babel-execute:plantuml) Include prologue and
  header variables to temporary file body.
(org-babel-plantuml-make-body): New function.  Return content of
temporary file used as input to PlantUML program.
(org-babel-variable-assignments:plantuml): New function.  Build list of
variable assignments for source block.

* testing/lisp/test-ob-plantuml.el: New file.  Test body text produced
  by `org-babel-plantuml-make-body'.
---
 lisp/ob-plantuml.el  | 28 ++-
 testing/lisp/test-ob-plantuml.el | 73 
 2 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 testing/lisp/test-ob-plantuml.el

diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
index 9ce65a9..01739c8 100644
--- a/lisp/ob-plantuml.el
+++ b/lisp/ob-plantuml.el
@@ -46,6 +46,31 @@
   :version "24.1"
   :type 'string)
 
+(defun org-babel-variable-assignments:plantuml (params)
+  "Return a list of PlantUML statements assigning the block's variables.
+PARAMS is a property list of source block parameters, which may
+contain multiple entries for the key `:var'.  `:var' entries in PARAMS
+are expected to be scalar variables."
+  (mapcar
+   (lambda (pair)
+   (format "!define %s %s"
+	   (car pair)
+	   (replace-regexp-in-string "\"" "" (cdr pair
+   (org-babel--get-vars params)))
+
+(defun org-babel-plantuml-make-body (body params)
+  "Return PlantUML input string.
+BODY is the content of the source block and PARAMS is a property list
+of source block parameters.  This function relies on the
+`org-babel-expand-body:generic' function to extract `:var' entries
+from PARAMS and on the `org-babel-variable-assignments:plantuml'
+function to convert variables to PlantUML assignments."
+  (concat
+   "@startuml\n"
+   (org-babel-expand-body:generic
+body params (org-babel-variable-assignments:plantuml params))
+   "\n@enduml"))
+
 (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'."
@@ -54,6 +79,7 @@ This function is called by `org-babel-execute-src-block'."
 	 (cmdline (cdr (assq :cmdline params)))
 	 (in-file (org-babel-temp-file "plantuml-"))
 	 (java (or (cdr (assq :java params)) ""))
+	 (full-body (org-babel-plantuml-make-body body params))
 	 (cmd (if (string= "" org-plantuml-jar-path)
 		  (error "`org-plantuml-jar-path' is not set")
 		(concat "java " java " -jar "
@@ -85,7 +111,7 @@ This function is called by `org-babel-execute-src-block'."
 			(org-babel-process-file-name out-file)
 (unless (file-exists-p org-plantuml-jar-path)
   (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
-(with-temp-file in-file (insert (concat "@startuml\n" body "\n@enduml")))
+(with-temp-file in-file (insert 

Re: [O] Add preamble support to ob-plantuml.el

[Nicolas Goaziou]

Hello,

Thibault Marin  writes:

> However, ob-plantuml does not seem to support the prologue option.  So I
> am modifying my patch to add support for the :prologue and :epilogue
> header arguments instead of using a new customization variable.  In the
> process, I have added support for header variables which are passed to
> the PlantUML file via the !define macro.  I am also adding a test file
> which checks that the temporary file passed to the plantuml program is
> properly generated (it does not run or check the output of plantuml).

Thank you.

> Please let me know you have any comment on the patch.  Thanks in
> advance.

Comments follow.

> +(defun org-babel-plantuml-var-to-plantuml (var)
> +  "Cleanup plantuml variable (remove quotes)."
> + (replace-regexp-in-string "\"" "" var))

Since this function is used only once in the code, I suggest to not
implement it and use `replace-regexp-in-string' at the appropriate
place.

> +(defun org-babel-variable-assignments:plantuml (params)
> +  "Return a list of PlantUML statements assigning the block's variables."

Could you document what is PARAMS?

> +  (mapcar
> +   (lambda (pair)
> +   (format "!define %s %s"
> +(car pair)
> +(org-babel-plantuml-var-to-plantuml (cdr pair
> +   (org-babel--get-vars params)))
> +
> +(defun org-babel-plantuml-make-body (body params)
> +  "Form PlantUML input string."

Do you mean "Return PlantUML" input string? Also you need to specify
what are body and params.

Besides, the same applies to `org-babel-plantuml-var-to-plantuml' above.
Is this function really needed, as it is a mere `format'.


Regards,

-- 
Nicolas Goaziou

Re: [O] Add preamble support to ob-plantuml.el

[Thibault Marin]

Thanks for the feedback.


>> What about using noweb syntax then?
>
> Or prologue?

I didn't know about the prologue header.  This is exactly what I need
(it seems to be more convenient than the noweb approach).

However, ob-plantuml does not seem to support the prologue option.  So I
am modifying my patch to add support for the :prologue and :epilogue
header arguments instead of using a new customization variable.  In the
process, I have added support for header variables which are passed to
the PlantUML file via the !define macro.  I am also adding a test file
which checks that the temporary file passed to the plantuml program is
properly generated (it does not run or check the output of plantuml).

Please let me know you have any comment on the patch.  Thanks in
advance.

I have a slightly tangential question about handling of list variables.

Considering the following org content:

#+BEGIN_SRC org
,#+name: variable_values
| CLASSNAME | test_class |
| PROPERTY  | test_prop  |

,#+header: :file tmp.puml
,#+header: :var x=variable_values
,#+begin_src plantuml
class CLASSNAME {
+PROPERTY
}
,#+end_src"
#+END_SRC

would it make sense to resolve to the following output:

#+BEGIN_SRC plantuml
@startuml
!define CLASSNAME test_class
!define PROPERTY test_prop
class CLASSNAME {
+PROPERTY
}
@enduml
#+END_SRC
?

On the first hand, this doesn't make much sense because the =x= variable is
never actually defined in the source block, but for languages that don't handle
array variables, that would allow the user to define multiple variables at once
(I first thought about this when looking at ob-css, where I wanted to (1) use
header variables and (2) group them in a single org-table).  What do you think?

Best,
thibault

>From 40e27d7e36806694c47fe4331b24db48c19f31f4 Mon Sep 17 00:00:00 2001
From: thibault 
Date: Tue, 6 Dec 2016 17:29:55 -0600
Subject: [PATCH] ob-plantuml.el: Add support for prologue and header variables

* lisp/ob-plantuml.el (org-babel-execute:plantuml) Include prologue and
  header variables to temporary file body.
(org-babel-plantuml-make-body): New function.  Form content of temporary
file used as input to PlantUML program.
(org-babel-plantuml-var-to-plantuml): New function.  Convert header
variable to PlantUML variable.  This reduces to removing quotes from
string.
(org-babel-variable-assignments:plantuml): New function.  Build list of
variable assignments for source block.

* testing/lisp/test-ob-plantuml.el: New file.  Test body text produced
  by `org-babel-plantuml-make-body'.
---
 lisp/ob-plantuml.el  | 24 -
 testing/lisp/test-ob-plantuml.el | 73 
 2 files changed, 96 insertions(+), 1 deletion(-)
 create mode 100644 testing/lisp/test-ob-plantuml.el

diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
index 9ce65a9..5b7ae20 100644
--- a/lisp/ob-plantuml.el
+++ b/lisp/ob-plantuml.el
@@ -46,6 +46,27 @@
   :version "24.1"
   :type 'string)
 
+(defun org-babel-plantuml-var-to-plantuml (var)
+  "Cleanup plantuml variable (remove quotes)."
+ (replace-regexp-in-string "\"" "" var))
+
+(defun org-babel-variable-assignments:plantuml (params)
+  "Return a list of PlantUML statements assigning the block's variables."
+  (mapcar
+   (lambda (pair)
+   (format "!define %s %s"
+	   (car pair)
+	   (org-babel-plantuml-var-to-plantuml (cdr pair
+   (org-babel--get-vars params)))
+
+(defun org-babel-plantuml-make-body (body params)
+  "Form PlantUML input string."
+  (concat
+   "@startuml\n"
+   (org-babel-expand-body:generic
+body params (org-babel-variable-assignments:plantuml params))
+   "\n@enduml"))
+
 (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'."
@@ -54,6 +75,7 @@ This function is called by `org-babel-execute-src-block'."
 	 (cmdline (cdr (assq :cmdline params)))
 	 (in-file (org-babel-temp-file "plantuml-"))
 	 (java (or (cdr (assq :java params)) ""))
+	 (full-body (org-babel-plantuml-make-body body params))
 	 (cmd (if (string= "" org-plantuml-jar-path)
 		  (error "`org-plantuml-jar-path' is not set")
 		(concat "java " java " -jar "
@@ -85,7 +107,7 @@ This function is called by `org-babel-execute-src-block'."
 			(org-babel-process-file-name out-file)
 (unless (file-exists-p org-plantuml-jar-path)
   (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
-(with-temp-file in-file (insert (concat "@startuml\n" body "\n@enduml")))
+(with-temp-file in-file (insert full-body))
 (message "%s" cmd) (org-babel-eval cmd "")
 nil)) ;; signal that output has already been written to file
 
diff --git a/testing/lisp/test-ob-plantuml.el b/testing/lisp/test-ob-plantuml.el
new file mode 100644
index 000..794d313
--- /dev/null
+++ b/testing/lisp/test-ob-plantuml.el
@@ -0,0 +1,73 @@
+;;; test-ob-plantuml.el --- tests for ob-plantuml.el
+
+;; 

Re: [O] Add preamble support to ob-plantuml.el

[Rainer M Krug]

Nicolas Goaziou  writes:

> Thibault Marin  writes:
>
>> I use it to define common skin options (http://plantuml.com/skinparam)
>> for all the plantuml blocks in an org file.  I don't know if there is a
>> better way to that.
>
> What about using noweb syntax then?

Or prologue?

,
| 14.8.2.28 ‘:prologue’
| .
| 
| The value of the ‘prologue’ header argument will be prepended to the
| code block body before execution.  For example, ‘:prologue "reset"’ may
| be used to reset a gnuplot session before execution of a particular code
| block, or the following configuration may be used to do this for all
| gnuplot code blocks.  Also see *note epilogue::.
`

But apart from this: I think it would be in general a good idea to add
version infos of emacs and org to source code blocks - possibly even of
the executable? This would be one more step towards easy
reproducability.

Cheers,

Rainer

>
> Regards,
>

-- 
Rainer M. Krug
email: Rainerkrugsde
PGP: 0x0F52F982


signature.asc
Description: PGP signature

Re: [O] Add preamble support to ob-plantuml.el

[Nicolas Goaziou]

Thibault Marin  writes:

> I use it to define common skin options (http://plantuml.com/skinparam)
> for all the plantuml blocks in an org file.  I don't know if there is a
> better way to that.

What about using noweb syntax then?

Regards,

Re: [O] Add preamble support to ob-plantuml.el

[Thibault Marin]

Hi, thanks for the feedback.

> You don't need to use the TINYCHANGE string since you signed FSF papers
> already.
Fixed.

> The :version keyword is inaccurate. It should be :version "25.2". It is
> also missing :package-version and :safe #'stringp.
Fixed (I hope).

> OOC, what is your use case?
I use it to define common skin options (http://plantuml.com/skinparam)
for all the plantuml blocks in an org file.  I don't know if there is a
better way to that.

Thanks,

thibault


>From 55edfde3636a9e558fe6ca1099477611a2f3ed0f Mon Sep 17 00:00:00 2001
From: thibault 
Date: Mon, 5 Dec 2016 15:46:46 -0600
Subject: [PATCH] ob-plantuml.el: Add preamble to PlantUML source block

* lisp/ob-plantuml.el (org-babel-execute:plantuml) Include preamble
  given by the new `org-plantuml-preamble' customization variable.
---
 lisp/ob-plantuml.el | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
index 9ce65a9..d73a04b 100644
--- a/lisp/ob-plantuml.el
+++ b/lisp/ob-plantuml.el
@@ -46,6 +46,14 @@
   :version "24.1"
   :type 'string)
 
+(defcustom org-plantuml-preamble ""
+  "Preamble added at the top of every plantuml source block."
+  :group 'org-babel
+  :version "25.2"
+  :package-version '(Org . "9.1")
+  :safe #'stringp
+  :type 'string)
+
 (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'."
@@ -85,7 +93,8 @@ This function is called by `org-babel-execute-src-block'."
 			(org-babel-process-file-name out-file)
 (unless (file-exists-p org-plantuml-jar-path)
   (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
-(with-temp-file in-file (insert (concat "@startuml\n" body "\n@enduml")))
+(with-temp-file in-file (insert (concat "@startuml\n" org-plantuml-preamble
+	"\n" body "\n@enduml")))
 (message "%s" cmd) (org-babel-eval cmd "")
 nil)) ;; signal that output has already been written to file
 
-- 
2.9.3

Re: [O] Add preamble support to ob-plantuml.el

[Nicolas Goaziou]

Hello,

Thibault Marin  writes:

> I am attaching a patch adding support for preamble commands to
> ob-plantuml.el.  The string content of the `org-plantuml-preamble'
> variable is added at the beginning of each source block before execution
> (after the "@startuml" string).

Thank you. Some comments follow.

> TINYCHANGE

You don't need to use the TINYCHANGE string since you signed FSF papers
already.

> ---
>  lisp/ob-plantuml.el | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
> index 9ce65a9..6463585 100644
> --- a/lisp/ob-plantuml.el
> +++ b/lisp/ob-plantuml.el
> @@ -46,6 +46,12 @@
>:version "24.1"
>:type 'string)
>  
> +(defcustom org-plantuml-preamble ""
> +  "Preamble added at the top of every plantuml source block."
> +  :group 'org-babel
> +  :version "24.1"
> +  :type 'string)

The :version keyword is inaccurate. It should be :version "25.2". It is
also missing :package-version and :safe #'stringp.

OOC, what is your use case?

Regards,

-- 
Nicolas Goaziou