Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Wed, Oct 04 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
>> Attached is:
>>
>> - my previous patch, rebased against the latest HEAD and only modified
>>   to include a patch to etc/ORG-NEWS
>> - a patch to the worg docs that corrects the urls
>> - a patch to the worg docs that documents the new header arguments and
>>   provides examples of usage (two new graphics files are included as
>>   part of that patch).
>
> Thanks!
> Applied, onto main.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=2ba45fcb7
>
> Updated Worg as well:
> https://git.sr.ht/~bzg/worg/commit/16ef094a
> https://git.sr.ht/~bzg/worg/commit/525a1355

Thanks, Ihor, for your time and feedback.

Leo

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Ihor Radchenko]

Leo Butler  writes:

> Attached is:
>
> - my previous patch, rebased against the latest HEAD and only modified
>   to include a patch to etc/ORG-NEWS
> - a patch to the worg docs that corrects the urls
> - a patch to the worg docs that documents the new header arguments and
>   provides examples of usage (two new graphics files are included as
>   part of that patch).

Thanks!
Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=2ba45fcb7

Updated Worg as well:
https://git.sr.ht/~bzg/worg/commit/16ef094a
https://git.sr.ht/~bzg/worg/commit/525a1355

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Fri, Sep 22 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
>>> What is the purpose of this `unwind-protect'?
>>> In its current form, it does nothing because no UNWINDFORMS are
>>> provided.
>>
>> I copied the forms from tests I had written for ob-octave, without
>> thinking about why those tests were wrapped in an `unwind-protect'
>> form. The attached patch removes them, otherwise it is identical to the
>> previous patch.
>
> Thanks! Now, may you also add etc/ORG-NEWS entry and update the WORG docs?

Attached is:

- my previous patch, rebased against the latest HEAD and only modified
  to include a patch to etc/ORG-NEWS
- a patch to the worg docs that corrects the urls
- a patch to the worg docs that documents the new header arguments and
  provides examples of usage (two new graphics files are included as
  part of that patch).

Best regards,
Leo

From 7288eb547aff1ad50c3782d9db22b1e2792d6634 Mon Sep 17 00:00:00 2001
From: Leo Butler 
Date: Tue, 19 Sep 2023 13:36:06 -0500
Subject: [PATCH] * lisp/ob-maxima.el: enable use of `batch' loader and `draw'

* (org-babel-header-args:maxima): Document the two new header
arguments (batch and graphics-pkg) that are specific to ob-maxima.
* (org-babel-maxima--command-arguments-default): A new variable
storing the default command-line argument(s).  This value was
hard-coded in `org-babel-maxima:execute'.
* (org-babel-maxima--graphic-package-options): A new variable that
stores an alist of Maxima graphics packages and the Maxima code to set
up that package.
* (org-babel-maxima--default-epilogue): A new variable that stores an
alist of the clean-up code that is run at end of a `graphical-output'
or `non-graphical-output' source block.
* (org-babel-maxima--output-filter-regexps): A new variable that
stores a list of regexps to identify "bad" output lines to be removed
from Maxima's output.  Source-code comments document each regexp's
purpose.  Two additional regexps have been added: one filters
un-wanted output from `batch' and the other removes empty input lines
that `batch'-ed output may spuriously produce (actual empty input
lines are syntax errors, see the new tests below).
* (org-babel-maxima--output-filter): A new function that takes a
single line of Maxima output.  It returns nil if the line has zero
length or matches a regexp in
`org-babel-maxima--output-filter-regexps'; otherwise, it returns the
line.  This function and regexp replace the hard-coded filter in
`org-babel-execute:maxima'.
* (org-babel-maxima-expand): Prepare the source block for execution,
depending on whether it is producing graphical output or not.  In case
of graphical output, use the `graphics-pkg' header to set the graphics
package and use `org-babel-maxima--graphic-package-options' to set-up
the package.  Grovel the graphics terminal from the output filename.
* (org-babel-execute:maxima): Use the :batch header argument and
`org-babel-maxima--command-arguments-default' to execute the source
block.  Replace the existing, in-line output filter and its regexps
with `org-babel-maxima--output-filter' and
`org-babel-maxima--output-filter-regexps'.

* testing/examples/ob-maxima-test.org: Add test examples.

Include examples of the batch-related tests from
testing/lisp/test-ob-maxima.el.  Provide an example of the
`:graphics-pkg' header argument with the `draw' package.

* testing/lisp/test-ob-maxima.el: Introduce six new, batch-related
test functions.  Each test exercises the :batch header argument.  The
response to unusual inputs is tested (empty strings, strings with just
whitespace, input with the `:lisp' reader, and two syntax-related
errors).

link: https://list.orgmode.org/87cyz1ivzw@t14.reltub.ca/
---
 etc/ORG-NEWS|  12 +++
 lisp/ob-maxima.el   | 115 +---
 testing/examples/ob-maxima-test.org |  54 +
 testing/lisp/test-ob-maxima.el  | 101 
 4 files changed, 254 insertions(+), 28 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 252c5a9f9..16f8a1c30 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -596,6 +596,18 @@ return a matplotlib Figure object to plot. For output results, the
 current figure (as returned by =pyplot.gcf()=) is cleared before
 evaluation, and then plotted afterwards.
 
+*** =ob-maxima.el=: Support for ~batch~ and ~draw~
+
+=ob-maxima= has two new header arguments: ~:batch~ and
+~:graphics-pkg~.
+
+The ~:batch~ header argument can be set to one of Maxima's file
+loaders (~batch~, ~load~ or ~batchload~); the default remains
+~batchload~. The ~:graphics-pkg~ header argument can be set to one of
+Maxima's graphics packages (~draw~ or ~plot~); the default remains
+~plot~.  The graphics terminal is now determined from the file-ending
+of the file-name set in the ~:file~ header argument.
+
 ** New functions and changes in function arguments
 *** =TYPES= argument in ~org-element-lineage~ can now be a symbol
 
diff --git a/lisp/ob-maxima.el 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Ihor Radchenko]

Leo Butler  writes:

>> What is the purpose of this `unwind-protect'?
>> In its current form, it does nothing because no UNWINDFORMS are
>> provided.
>
> I copied the forms from tests I had written for ob-octave, without
> thinking about why those tests were wrapped in an `unwind-protect'
> form. The attached patch removes them, otherwise it is identical to the
> previous patch.

Thanks! Now, may you also add etc/ORG-NEWS entry and update the WORG docs?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Thu, Sep 21 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
 +(defconst org-babel-header-args:maxima
 +  '((batch   . :any)
>>>
>>> Why :any? Only two values are allowed here.
>>>
 +(graphics-pkg. :any))
>>>
>>> Same. The value is a closed list.
>>
>> I have made the change requested.
>>
>> However, beyond the documentation value, I don't see this variable's
>> values being used.
>
> In completion. See `pcomplete/org-mode/block-option/src'.
> Also, in org-lint. See `org-lint-wrong-header-argument'.

Thanks.

>
>> Fixed. I don't use `apropos', so I didn't see the problem (and I had
>> failed to absorb everything in §D.6).
>
> M-x checkdoc checks for many of the §D.6 conventions.

Yes, I discovered that from the documentation. I must have done
something wrong, because it didn't work for me when I tried it, but
after re-starting emacs it is working.

>
>> Please see the attached, revised patch. It fixes the problems that you
>> have identified–except for the way that the gnuplot terminal is
>> determined. I would like to leave that for a subsequent patch, mainly
>> because this one has become quite large.
>
> Ok. Thanks!
>
> I have one more comment on the tests:
>
>> +(ert-deftest ob-maxima/batch+verbatim+eof-error ()
>> +  "Exercise the `:batch' header argument.  Send invalid input to
>> +Maxima."
>> +  (unwind-protect
>
> What is the purpose of this `unwind-protect'?
> In its current form, it does nothing because no UNWINDFORMS are
> provided.

I copied the forms from tests I had written for ob-octave, without
thinking about why those tests were wrapped in an `unwind-protect'
form. The attached patch removes them, otherwise it is identical to the
previous patch.

Leo

From 425ea8ed47af5bccfd9bc37f1e4de4c53cc9b1f1 Mon Sep 17 00:00:00 2001
From: Leo Butler 
Date: Tue, 19 Sep 2023 13:36:06 -0500
Subject: [PATCH] * lisp/ob-maxima.el: enable use of `batch' loader and `draw'

* (org-babel-header-args:maxima): Document the two new header
arguments (batch and graphics-pkg) that are specific to ob-maxima.
* (org-babel-maxima--command-arguments-default): A new variable
storing the default command-line argument(s).  This value was
hard-coded in `org-babel-maxima:execute'.
* (org-babel-maxima--graphic-package-options): A new variable that
stores an alist of Maxima graphics packages and the Maxima code to set
up that package.
* (org-babel-maxima--default-epilogue): A new variable that stores an
alist of the clean-up code that is run at end of a `graphical-output'
or `non-graphical-output' source block.
* (org-babel-maxima--output-filter-regexps): A new variable that
stores a list of regexps to identify "bad" output lines to be removed
from Maxima's output.  Source-code comments document each regexp's
purpose.  Two additional regexps have been added: one filters
un-wanted output from `batch' and the other removes empty input lines
that `batch'-ed output may spuriously produce (actual empty input
lines are syntax errors, see the new tests below).
* (org-babel-maxima--output-filter): A new function that takes a
single line of Maxima output.  It returns nil if the line has zero
length or matches a regexp in
`org-babel-maxima--output-filter-regexps'; otherwise, it returns the
line.  This function and regexp replace the hard-coded filter in
`org-babel-execute:maxima'.
* (org-babel-maxima-expand): Prepare the source block for execution,
depending on whether it is producing graphical output or not.  In case
of graphical output, use the `graphics-pkg' header to set the graphics
package and use `org-babel-maxima--graphic-package-options' to set-up
the package.  Grovel the graphics terminal from the output filename.
* (org-babel-execute:maxima): Use the :batch header argument and
`org-babel-maxima--command-arguments-default' to execute the source
block.  Replace the existing, in-line output filter and its regexps
with `org-babel-maxima--output-filter' and
`org-babel-maxima--output-filter-regexps'.

* testing/examples/ob-maxima-test.org: Add test examples.

Include examples of the batch-related tests from
testing/lisp/test-ob-maxima.el.  Provide an example of the
`:graphics-pkg' header argument with the `draw' package.

* testing/lisp/test-ob-maxima.el: Introduce six new, batch-related
test functions.  Each test exercises the :batch header argument.  The
response to unusual inputs is tested (empty strings, strings with just
whitespace, input with the `:lisp' reader, and two syntax-related
errors).
---
 lisp/ob-maxima.el   | 115 +---
 testing/examples/ob-maxima-test.org |  54 +
 testing/lisp/test-ob-maxima.el  | 101 
 3 files changed, 242 insertions(+), 28 deletions(-)

diff --git a/lisp/ob-maxima.el b/lisp/ob-maxima.el
index d1d7c7424..eac44ff9a 100644
--- a/lisp/ob-maxima.el
+++ b/lisp/ob-maxima.el
@@ -37,6 +37,11 @@
 
 (require 'ob)
 
+(defconst org-babel-header-args:maxima
+  '((batch   . 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Ihor Radchenko]

Leo Butler  writes:

>>> +(defconst org-babel-header-args:maxima
>>> +  '((batch   . :any)
>>
>> Why :any? Only two values are allowed here.
>>
>>> +(graphics-pkg. :any))
>>
>> Same. The value is a closed list.
>
> I have made the change requested.
>
> However, beyond the documentation value, I don't see this variable's
> values being used.

In completion. See `pcomplete/org-mode/block-option/src'.
Also, in org-lint. See `org-lint-wrong-header-argument'.

> Fixed. I don't use `apropos', so I didn't see the problem (and I had
> failed to absorb everything in §D.6).

M-x checkdoc checks for many of the §D.6 conventions.

> Please see the attached, revised patch. It fixes the problems that you
> have identified–except for the way that the gnuplot terminal is
> determined. I would like to leave that for a subsequent patch, mainly
> because this one has become quite large.

Ok. Thanks!

I have one more comment on the tests:

> +(ert-deftest ob-maxima/batch+verbatim+eof-error ()
> +  "Exercise the `:batch' header argument.  Send invalid input to
> +Maxima."
> +  (unwind-protect

What is the purpose of this `unwind-protect'?
In its current form, it does nothing because no UNWINDFORMS are
provided.

> +(org-test-with-temp-text
> +(format "#+name: ob-maxima/batch+verbatim+eof-error
> +#+begin_src maxima  :results verbatim :batch batch :cmdline --quiet
> +x:
> +#+end_src
> +")
> +  (should (string-match "end of file while scanning expression\\."
> +(org-babel-execute-src-block))



-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Wed, Sep 20 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
> May you clarify the purpose of "linenum"?
>>> Do I understand correctly that the above will simply affect debug output
>>> when maxima references where a problematic line is located in the source?
>>
>> No, it affects how output labels are printed. With this value, the
>> "first" line in the source block receives the line number 1; without it,
>> it would get 2.
>
> Ok. This should be documented in the commit message and possibly in the
> code itself.

I think it makes most sense to put in a source-code comment, explaining
the why. That is what I have done.

>
>>>
>>(unless (or (string-match "batch" line)
>>(string-match "^rat: replaced 
>> .*$" line)
>>(string-match "^;;; Loading 
>> #P" line)
>> +  (string-match "^read and 
>> interpret" line)
>> +  (string-match 
>> "^(%\\([io]-?[0-9]+\\))[ ]+$" line)
>
> May you explain why you added these two conditions?

>>
>> 
>>
>>>
>>> May empty lines be intentional in some maxima code?
>>
>> A blank line is simply skipped by the maxima reader; an empty input line
>> is a syntax error.
>>
>> However, "empty" output may appear, so I have modified that filter.
>
>+  (string-match 
> "^(%\\([io]-?[0-9]+\\))[ ]+$" line)
>
> It would be nice to add a comment line explaining what this regexp
> matches against.

Ok. I think it makes the most sense to explain the purpose of each
regexp, new and old, and to make this somewhat extensible.

What I have done is cut out that undocumented filter, put the regexps
into a new variable (`org-babel-maxima--output-filter-regexps'), added
source-code comments to explain what each one does and added a new
function (`org-babel-maxima--output-filter') that replaces the old
filter in `org-babel-execute:maxima'.


>
>> * (org-babel-maxima--graphic-package-options): an alist of Maxima
>> graphics packages and the Maxima code to set up that package.
>
> This is a bit confusing. I'd say "A new variable storing alist of Maxima ..."
>
>> * (org-babel-maxima--default-epilogue): an alist of the clean-up code
>> that is run at end of a `graphical-output' or `non-graphical-output'
>> source block.
>
> Same here.
> Basically, it should be clear from the commit message when you add new
> variables and when you modify existing.

Points taken.

>
>> * (org-babel-maxima-expand): prepare the source block for
>> execution.
>
> This is vague. Please explain what is changed in more details.

Done.

>
>> * (org-babel-execute:maxima): use the :batch header argument and
>> `org-babel-maxima--command-arguments-default' to execute the source
>> block.  Add a couple extra regexps to filter the output of a batch-ed
>> source block.
>
> This reads better compared to the above.
>
>> +(defconst org-babel-header-args:maxima
>> +  '((batch   . :any)
>
> Why :any? Only two values are allowed here.
>
>> +(graphics-pkg. :any))
>
> Same. The value is a closed list.

I have made the change requested.

However, beyond the documentation value, I don't see this variable's
values being used.

>
>> +(defvar org-babel-maxima--command-arguments-default
>> +  "--very-quiet"
>> +  "Command-line arguments sent to Maxima by default.  If the
>
> Please keep the first line as a single sentence.
> See 
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html
>
>> +(defvar org-babel-maxima--default-epilogue
>> +  '((graphical-output . "gnuplot_close ()$")
>> +(non-graphical-output . ""))
>> +  "The final Maxima code executed in a source block.  An alist with
>
> Same problem with the first line.

Fixed. I don't use `apropos', so I didn't see the problem (and I had
failed to absorb everything in §D.6).

Please see the attached, revised patch. It fixes the problems that you
have identified–except for the way that the gnuplot terminal is
determined. I would like to leave that for a subsequent patch, mainly
because this one has become quite large.

Best regards,
Leo

From 078e29f13f15305e291b1c0e46e306d633a8b93a Mon Sep 17 00:00:00 2001
From: Leo Butler 
Date: Tue, 19 Sep 2023 13:36:06 -0500
Subject: [PATCH] * lisp/ob-maxima.el: enable use of `batch' loader and `draw'

* (org-babel-header-args:maxima): Document the two new header
arguments (batch and graphics-pkg) that are specific to ob-maxima.
* (org-babel-maxima--command-arguments-default): A new variable
storing the default command-line argument(s).  This value was
hard-coded in `org-babel-maxima:execute'.
* (org-babel-maxima--graphic-package-options): A new variable that
stores an alist of Maxima graphics packages and the Maxima code to set
up that package.
* (org-babel-maxima--default-epilogue): A new variable that 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Ihor Radchenko]

Leo Butler  writes:

 May you clarify the purpose of "linenum"?
>> Do I understand correctly that the above will simply affect debug output
>> when maxima references where a problematic line is located in the source?
>
> No, it affects how output labels are printed. With this value, the
> "first" line in the source block receives the line number 1; without it,
> it would get 2.

Ok. This should be documented in the commit message and possibly in the
code itself.

>>
>(unless (or (string-match "batch" line)
>(string-match "^rat: replaced 
> .*$" line)
>(string-match "^;;; Loading 
> #P" line)
> +  (string-match "^read and 
> interpret" line)
> +  (string-match 
> "^(%\\([io]-?[0-9]+\\))[ ]+$" line)

 May you explain why you added these two conditions?
>>>
>
> 
>
>>
>> May empty lines be intentional in some maxima code?
>
> A blank line is simply skipped by the maxima reader; an empty input line
> is a syntax error.
>
> However, "empty" output may appear, so I have modified that filter.

   +  (string-match 
"^(%\\([io]-?[0-9]+\\))[ ]+$" line)

It would be nice to add a comment line explaining what this regexp
matches against.

> * (org-babel-maxima--graphic-package-options): an alist of Maxima
> graphics packages and the Maxima code to set up that package.

This is a bit confusing. I'd say "A new variable storing alist of Maxima ..."

> * (org-babel-maxima--default-epilogue): an alist of the clean-up code
> that is run at end of a `graphical-output' or `non-graphical-output'
> source block.

Same here.
Basically, it should be clear from the commit message when you add new
variables and when you modify existing.

> * (org-babel-maxima-expand): prepare the source block for
> execution.

This is vague. Please explain what is changed in more details.

> * (org-babel-execute:maxima): use the :batch header argument and
> `org-babel-maxima--command-arguments-default' to execute the source
> block.  Add a couple extra regexps to filter the output of a batch-ed
> source block.

This reads better compared to the above.

> +(defconst org-babel-header-args:maxima
> +  '((batch   . :any)

Why :any? Only two values are allowed here.

> +(graphics-pkg. :any))

Same. The value is a closed list.

> +(defvar org-babel-maxima--command-arguments-default
> +  "--very-quiet"
> +  "Command-line arguments sent to Maxima by default.  If the

Please keep the first line as a single sentence.
See 
https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html

> +(defvar org-babel-maxima--default-epilogue
> +  '((graphical-output . "gnuplot_close ()$")
> +(non-graphical-output . ""))
> +  "The final Maxima code executed in a source block.  An alist with

Same problem with the first line.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Sat, Sep 16 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
>>> Also, non-standard arguments should be defined in 
>>> `org-babel-header-args:maxima'.
>>>
>>
>> Ok. I see that some packages (e.g. ob-gnuplot.el) use a `defvar' form,
>> while others (e.g. ob-R.el) use a `defconst' form. Is there a preference?
>
> defconst I think. The value is not supposed to be changed.

Done.

>
 I have also moved two defaults, that were embedded in the code, to
 `defvar' forms.
>>>
>>> This is fine, although I would prefer to keep these variables private for
>>> now.

Done.

 -  (format "batchload(%S)$" in-file))
 +  (format "(linenum:0, %s(%S))$" batch/load 
 in-file))
>>>
>>> May you clarify the purpose of "linenum"?
> Do I understand correctly that the above will simply affect debug output
> when maxima references where a problematic line is located in the source?

No, it affects how output labels are printed. With this value, the
"first" line in the source block receives the line number 1; without it,
it would get 2.

>
(unless (or (string-match "batch" line)
(string-match "^rat: replaced 
 .*$" line)
(string-match "^;;; Loading #P" 
 line)
 +  (string-match "^read and 
 interpret" line)
 +  (string-match 
 "^(%\\([io]-?[0-9]+\\))[ ]+$" line)
>>>
>>> May you explain why you added these two conditions?
>>



>
> May empty lines be intentional in some maxima code?

A blank line is simply skipped by the maxima reader; an empty input line
is a syntax error.

However, "empty" output may appear, so I have modified that filter. I
have included a number of test cases in the testsuite to make sure that
such corner cases are being handled correctly.

Please see the attached patch.

Thank you for your time and patience.

Leo

From d2934b38a28a8593554af69622be5b11bc13406d Mon Sep 17 00:00:00 2001
From: Leo Butler 
Date: Tue, 19 Sep 2023 13:36:06 -0500
Subject: [PATCH] * lisp/ob-maxima.el: enable use of `batch' loader and `draw'

* (org-babel-header-args:maxima): document the two
new header arguments, specific to ob-maxima.
* (org-babel-maxima--command-arguments-default):
move the default command-line argument into this variable, from
`org-babel-maxima:execute'.
* (org-babel-maxima--graphic-package-options): an alist of Maxima
graphics packages and the Maxima code to set up that package.
* (org-babel-maxima--default-epilogue): an alist of the clean-up code
that is run at end of a `graphical-output' or `non-graphical-output'
source block.
* (org-babel-maxima-expand): prepare the source block for
execution.
* (org-babel-execute:maxima): use the :batch header argument and
`org-babel-maxima--command-arguments-default' to execute the source
block.  Add a couple extra regexps to filter the output of a batch-ed
source block.

* testing/examples/ob-maxima-test.org: add a draw test

Provide an example of the `:graphics-pkg' header argument with the
`draw' package.

* testing/lisp/test-ob-maxima.el: add batch-related tests

* testing/lisp/test-ob-maxima.el: introduce six new tests.  Each test
exercises the :batch header argument.  The response to unusual inputs
is tested (empty strings, strings with just whitespace, input with the
`:lisp' reader, and two syntax-related errors).
* testing/examples/ob-maxima-test.org: include examples of the
batch-related tests from testing/lisp/test-ob-maxima.el.
---
 lisp/ob-maxima.el   |  88 +--
 testing/examples/ob-maxima-test.org |  54 ++
 testing/lisp/test-ob-maxima.el  | 107 
 3 files changed, 227 insertions(+), 22 deletions(-)

diff --git a/lisp/ob-maxima.el b/lisp/ob-maxima.el
index d1d7c7424..3814049ca 100644
--- a/lisp/ob-maxima.el
+++ b/lisp/ob-maxima.el
@@ -37,6 +37,11 @@
 
 (require 'ob)
 
+(defconst org-babel-header-args:maxima
+  '((batch   . :any)
+(graphics-pkg. :any))
+  "Maxima-specific header arguments.")
+
 (defvar org-babel-tangle-lang-exts)
 (add-to-list 'org-babel-tangle-lang-exts '("maxima" . "max"))
 
@@ -48,29 +53,59 @@
   :group 'org-babel
   :type 'string)
 
+(defvar org-babel-maxima--command-arguments-default
+  "--very-quiet"
+  "Command-line arguments sent to Maxima by default.  If the
+`:batch' header argument is set to `batchload' or unset, then the
+`:cmdline' header argument is appended to this default;
+otherwise, if the `:cmdline' argument is set, it over-rides this
+default.  See `org-babel-maxima-command' and
+`org-babel-execute:maxima'.")
+
+(defvar org-babel-maxima--graphic-package-options
+  '((plot . "(set_plot_option ('[gnuplot_term, %s]), set_plot_option ('[gnuplot_out_file, %S]))$")
+(draw . "(load(draw), 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Ihor Radchenko]

Leo Butler  writes:

>> Also, non-standard arguments should be defined in 
>> `org-babel-header-args:maxima'.
>>
>
> Ok. I see that some packages (e.g. ob-gnuplot.el) use a `defvar' form,
> while others (e.g. ob-R.el) use a `defconst' form. Is there a preference?

defconst I think. The value is not supposed to be changed.

>>> I have also moved two defaults, that were embedded in the code, to
>>> `defvar' forms.
>>
>> This is fine, although I would prefer to keep these variables private for
>> now.
>
> My understanding is that a special variable defined via `defvar' is one
> that is intended to be "private", i.e. users should not change it unless
> they really know what they are doing. Is that accurate?

Users are indeed not supposed to change defvar unless they know what
they are doing. But it is not considered private - there is a guarantee
that the meaning or type of the variable is not going to change.

> Do you mean the names should be something like
>
> org-babel-maxima--command-arguments
>
> ?

Yes. Having "--" means - if you use this variable, do it at your own
risk; we can re-purpose it or change or rename or do whatever at any
time.

>>> -  (format "batchload(%S)$" in-file))
>>> +  (format "(linenum:0, %s(%S))$" batch/load 
>>> in-file))
>>
>> May you clarify the purpose of "linenum"?
>
> Maxima keeps track of input/output line numbers via the variable
> `linenum'. I set the linenum to 0 so that the line numbering of the
> input in `in-file' starts at 1 not 2.
>
> This idiom has been used in other Maxima front-ends, such as
> `imaxima.el', too (although imaxima now uses the lisp reader, instead).
>
> See 
> https://sourceforge.net/p/maxima/code/ci/76105d9ee231679eccac888a04c98e6ef66df087/

Do I understand correctly that the above will simply affect debug output
when maxima references where a problematic line is located in the source?

>>>(unless (or (string-match "batch" line)
>>>(string-match "^rat: replaced 
>>> .*$" line)
>>>(string-match "^;;; Loading #P" 
>>> line)
>>> +  (string-match "^read and 
>>> interpret" line)
>>> +  (string-match 
>>> "^(%\\([io]-?[0-9]+\\))[ ]+$" line)
>>
>> May you explain why you added these two conditions?
>
> When `batch' starts, it emits the line starting with 'read and
> interpret' and including the name of the file being read. This is
> extraneous output, and should be filtered out.
>
> The second addition filters out empty input/output lines. Without that
> filter, the block
>
> #+name: ob-maxima/batch+verbatim+quiet
> #+begin_src maxima :exports both :results verbatim :batch batch :cmdline 
> --quiet
> (assume(z>0),
> integrate(exp(-t)*t^z, t, 0, inf));
> #+end_src
>
> produces an output with a pair of empty lines:
>
> #+RESULTS: ob-maxima/batch+verbatim+quiet
> : (%i1) (assume(z > 0),integrate(exp(-t)*t^z,t,0,inf))
> : (%o1)gamma(z + 1)
> : (%i2) 
> : (%i4) 
>
>
> I don't understand why those extra lines appear. It looks to me like a
> bug in Maxima, but, because of that, they need to be filtered out.

May empty lines be intentional in some maxima code?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Fri, Sep 15 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
>> Attached is a patch that tries to address some of Ihor's concerns. I
>> have added two header arguments for maxima src blocks:
>
> Thanks!
>
>> - :graphics-pkg lets the user choose the graphics package to use;
>>
>> - :batch lets the user choose which source-code loader Maxima will use.
>
> We need to document the new header arguments in ORG-NEWS and in
> https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-maxima.html

Thanks for your feedback.

Yes, I will do the documentation once the patch has attained a
nearer-to-finished state.

>
> Also, non-standard arguments should be defined in 
> `org-babel-header-args:maxima'.
>

Ok. I see that some packages (e.g. ob-gnuplot.el) use a `defvar' form,
while others (e.g. ob-R.el) use a `defconst' form. Is there a preference?

>> I have also moved two defaults, that were embedded in the code, to
>> `defvar' forms.
>
> This is fine, although I would prefer to keep these variables private for
> now.

My understanding is that a special variable defined via `defvar' is one
that is intended to be "private", i.e. users should not change it unless
they really know what they are doing. Is that accurate?

Do you mean the names should be something like

org-babel-maxima--command-arguments

?

>
>> +(defvar org-babel-maxima-command-arguments
>> +  "--very-quiet"
>> + "A string containing the command-line arguments used when calling
>> the Maxima executable. See `org-babel-maxima-command',
>> `org-babel-maxima-batch/load' and `org-babel-execute:maxima'.")
>
> Here and in the other docstrings, please use a single short sentence in
> the first line. Also, please use double space between sentences and fill
> the docstring text to standard fill-column. See
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html

Yes.

>
>> +(defvar org-babel-maxima-graphic-package-options
>> +  '((plot . "(set_plot_option ('[gnuplot_term, %s]), set_plot_option 
>> ('[gnuplot_out_file, %S]))$")
>> +(draw . "(load(draw), set_draw_defaults(terminal='%s,file_name=%S))$"))
>> + "An alist, each element of the form (PACKAGE-NAME
>> . FORMAT-STRING). The format string contains the Maxima code to set
>> the graphic file terminal and name. It must contain `%s' to set the
>> terminal and `%S' to set the filename. The default package is
>> `plot'. See `org-babel-maxima-expand'.")
>
> According to the docstring, it appears that the order of %s and %S do
> not matter, which is probably not the case.

Correct.

>
>> +  ;; graphic output
>> +  (if graphic-file
>> +  (let ((graphics-pkg (intern (or (cdr (assq 
>> :graphics-pkg params)) "plot")))
>> +(graphic-format-string (cdr (assq 
>> graphics-pkg org-babel-maxima-graphic-package-options)))
>> +(graphic-terminal (file-name-extension 
>> graphic-file))
>> +(graphic-file (if (eq graphics-pkg 'plot) 
>> graphic-file (file-name-sans-extension graphic-file
>> +(format graphic-format-string graphic-terminal 
>> graphic-file)))
>
> What will happen if :graphics-pkg value is not expected? What about
> unsupported graphics file extensions? 

I need to add a fail-safe when setting `graphic-format-string'

> And is it always the that terminal name is the same as file extension
> (I am thinking about Gnuplot's pngcairo terminal)?

I see your point.

At the moment, the code works for the following terminals:

png, jpg, gif, eps, svg, pdf

To support a range of terminal options including, I would need to add a
:terminal header argument and an alist of graphics packages and their
supported terminals.

>
>> -  (format "batchload(%S)$" in-file))
>> +  (format "(linenum:0, %s(%S))$" batch/load 
>> in-file))
>
> May you clarify the purpose of "linenum"?

Maxima keeps track of input/output line numbers via the variable
`linenum'. I set the linenum to 0 so that the line numbering of the
input in `in-file' starts at 1 not 2.

This idiom has been used in other Maxima front-ends, such as
`imaxima.el', too (although imaxima now uses the lisp reader, instead).

See 
https://sourceforge.net/p/maxima/code/ci/76105d9ee231679eccac888a04c98e6ef66df087/


>
>>(unless (or (string-match "batch" line)
>>(string-match "^rat: replaced 
>> .*$" line)
>>(string-match "^;;; Loading #P" 
>> line)
>> +  (string-match "^read and 
>> interpret" line)
>> +  (string-match 
>> "^(%\\([io]-?[0-9]+\\))[ ]+$" line)
>
> May you explain why you added these two conditions?

When `batch' starts, it emits the line starting with 'read and
interpret' and including the name of 

Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Ihor Radchenko]

Leo Butler  writes:

> Attached is a patch that tries to address some of Ihor's concerns. I
> have added two header arguments for maxima src blocks:

Thanks!

> - :graphics-pkg lets the user choose the graphics package to use;
>
> - :batch lets the user choose which source-code loader Maxima will use.

We need to document the new header arguments in ORG-NEWS and in
https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-maxima.html

Also, non-standard arguments should be defined in 
`org-babel-header-args:maxima'.

> I have also moved two defaults, that were embedded in the code, to
> `defvar' forms.

This is fine, although I would prefer to keep these variables private for
now.

> +(defvar org-babel-maxima-command-arguments
> +  "--very-quiet"
> +  "A string containing the command-line arguments used when calling the 
> Maxima executable. See `org-babel-maxima-command', 
> `org-babel-maxima-batch/load' and `org-babel-execute:maxima'.")

Here and in the other docstrings, please use a single short sentence in
the first line. Also, please use double space between sentences and fill
the docstring text to standard fill-column. See
https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html

> +(defvar org-babel-maxima-graphic-package-options
> +  '((plot . "(set_plot_option ('[gnuplot_term, %s]), set_plot_option 
> ('[gnuplot_out_file, %S]))$")
> +(draw . "(load(draw), set_draw_defaults(terminal='%s,file_name=%S))$"))
> +  "An alist, each element of the form (PACKAGE-NAME . FORMAT-STRING). The 
> format string contains the Maxima code to set the graphic file terminal and 
> name. It must contain `%s' to set the terminal and `%S' to set the filename. 
> The default package is `plot'. See `org-babel-maxima-expand'.")

According to the docstring, it appears that the order of %s and %S do
not matter, which is probably not the case.

> +   ;; graphic output
> +   (if graphic-file
> +  (let ((graphics-pkg (intern (or (cdr (assq 
> :graphics-pkg params)) "plot")))
> +(graphic-format-string (cdr (assq 
> graphics-pkg org-babel-maxima-graphic-package-options)))
> +(graphic-terminal (file-name-extension 
> graphic-file))
> +(graphic-file (if (eq graphics-pkg 'plot) 
> graphic-file (file-name-sans-extension graphic-file
> +(format graphic-format-string graphic-terminal 
> graphic-file)))

What will happen if :graphics-pkg value is not expected? What about
unsupported graphics file extensions? And is it always the that terminal
name is the same as file extension (I am thinking about Gnuplot's
pngcairo terminal)?

> -  (format "batchload(%S)$" in-file))
> +  (format "(linenum:0, %s(%S))$" batch/load 
> in-file))

May you clarify the purpose of "linenum"?

>(unless (or (string-match "batch" line)
>(string-match "^rat: replaced .*$" 
> line)
>(string-match "^;;; Loading #P" 
> line)
> +  (string-match "^read and 
> interpret" line)
> +  (string-match 
> "^(%\\([io]-?[0-9]+\\))[ ]+$" line)

May you explain why you added these two conditions?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

[PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)

[Leo Butler]

On Thu, Sep 07 2023, Ihor Radchenko  wrote:

> `org-babel-execute:maxima' relies on certain output that maxima
> produces. Removing --very-quiet will make the assumptions in the code no
> longer valid. Worse - it might happen that in the absence of --very-quiet
> `org-babel-execute:maxima' (or its future code) will work _almost_
> correctly, with problems going unnoticed to the user.
>
> If we want to support use-case when --very-quiet is absent, we need to
> explicitly change `org-babel-execute:maxima' to account for it,
> maintaining this support forever.
>
> Either way, it will be an extra maintenance burden, which must be
> justified.
>
> The baseline is - we cannot put the burden of wrongly changing
> customization onto the user. Because the user may or may not notice
> them problem, especially when it is subtle and requires good knowledge
> of the Elisp code in ob-maxima.
>
> Of course, the above statement is not 100% strict. If you describe cases
> when customization is necessary for certain valid use cases, we may
> still put in such dangerous customization with all the appropriate
> warnings in the docstring. But it should be justified.
>
>>> So, leaving essential settings customizeable is not necessarily a good idea.
>>
>> I understand your hesitation about full-blown customization using
>> `defcustom'. However, I would still like to have more dials to turn.
>>
>> Perhaps we could add header arguments to get the desired customization?
>
> Header arguments are generally better, because they provide more
> fine-grained control compared to global customization.
>
>> E.g.
>>
>> - :batch :: Control how the Maxima source is evaluated by Maxima.
>>   1. Default. If nil or no, then use batchload with the --very-quiet
>>  command-line flag.
>>   2. If t or yes, then use batch with the --quiet command-line flag;
>
> Is there a place where I can see the differences between batch and batchload?
>
>> - :plot-engine :: Set the plotting package.
>>   1. Default. If nil or no, the use `plot';
>>   2. If `draw', then use `draw'.
>
> Sounds reasonable.
>
>> - Similarly, we could do something like ob-R.el does, and construct the
>>   graphics instructions using some additional header arguments and
>>   grovelling the terminal from the filename (see
>>   org-babel-R-construct-graphics-device-call).
>>
>> My sense is that this would be more in keeping with how other ob-*.el
>> packages do things.
>
> Yes.

Attached is a patch that tries to address some of Ihor's concerns. I
have added two header arguments for maxima src blocks:

- :graphics-pkg lets the user choose the graphics package to use;

- :batch lets the user choose which source-code loader Maxima will use.

I have also moved two defaults, that were embedded in the code, to
`defvar' forms.

I have added tests in test-ob-maxima.el and in ob-maxima-test.org to
demonstrate the use of these header arguments.

Leo


diff --git a/lisp/ob-maxima.el b/lisp/ob-maxima.el
index d1d7c7424..ab166bfd4 100644
--- a/lisp/ob-maxima.el
+++ b/lisp/ob-maxima.el
@@ -48,29 +48,44 @@
   :group 'org-babel
   :type 'string)
 
+(defvar org-babel-maxima-command-arguments
+  "--very-quiet"
+  "A string containing the command-line arguments used when calling the Maxima executable. See `org-babel-maxima-command', `org-babel-maxima-batch/load' and `org-babel-execute:maxima'.")
+
+(defvar org-babel-maxima-graphic-package-options
+  '((plot . "(set_plot_option ('[gnuplot_term, %s]), set_plot_option ('[gnuplot_out_file, %S]))$")
+(draw . "(load(draw), set_draw_defaults(terminal='%s,file_name=%S))$"))
+  "An alist, each element of the form (PACKAGE-NAME . FORMAT-STRING). The format string contains the Maxima code to set the graphic file terminal and name. It must contain `%s' to set the terminal and `%S' to set the filename. The default package is `plot'. See `org-babel-maxima-expand'.")
+
+(defvar org-babel-maxima-default-epilogue
+  "gnuplot_close ()$"
+  "A string with the final Maxima code executed. See `org-babel-maxima-expand'.")
+
 (defun org-babel-maxima-expand (body params)
   "Expand a block of Maxima code according to its header arguments."
-  (let ((vars (org-babel--get-vars params))
-	(epilogue (cdr (assq :epilogue params)))
-	(prologue (cdr (assq :prologue params
+  (let* ((vars (org-babel--get-vars params))
+ (graphic-file (ignore-errors (org-babel-graphical-output-file params)))
+	 (epilogue (cdr (assq :epilogue params)))
+	 (prologue (cdr (assq :prologue params
 (mapconcat 'identity
-	   (list
-		;; Any code from the specified prologue at the start.
-		prologue
-		;; graphic output
-		(let ((graphic-file (ignore-errors (org-babel-graphical-output-file params
-		  (if graphic-file
-		  (format
-		   "set_plot_option ([gnuplot_term, png]); set_plot_option ([gnuplot_out_file, %S]);"
-		   graphic-file)
-		""))
-		;; variables
-		(mapconcat 'org-babel-maxima-var-to-maxima vars "\n")
-		;; body
-		body
-		;; Any code from the