emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: Leo Butler <Leo.Butler@umanitoba.ca>
Cc: Bastien <bzg@gnu.org>,
	Lockywolf <for_org-bugs_2023-09-01@lockywolf.net>,
	"emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom)
Date: Fri, 15 Sep 2023 09:41:38 +0000	[thread overview]
Message-ID: <87jzsrai3x.fsf@localhost> (raw)
In-Reply-To: <87y1hb5cb4.fsf_-_@t14.reltub.ca>

Leo Butler <Leo.Butler@umanitoba.ca> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


  reply	other threads:[~2023-09-15  9:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-01  4:35 [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)] Lockywolf
2023-09-01 18:33 ` Leo Butler
2023-09-02  7:19   ` Ihor Radchenko
2023-09-02 18:12     ` Leo Butler
2023-09-05 10:57       ` [MAINTENANCE] On how much we can expose internals into defcustom (was: [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)]) Ihor Radchenko
2023-09-06 19:39         ` [MAINTENANCE] On how much we can expose internals into defcustom Leo Butler
2023-09-07 11:35           ` Ihor Radchenko
2023-09-12 21:09             ` [PATCH] ob-maxima.el, etc. (was Re: [MAINTENANCE] On how much we can expose internals into defcustom) Leo Butler
2023-09-15  9:41               ` Ihor Radchenko [this message]
2023-09-15 15:13                 ` Leo Butler
2023-09-16  9:04                   ` Ihor Radchenko
2023-09-19 19:25                     ` Leo Butler
2023-09-20  9:17                       ` Ihor Radchenko
2023-09-20 15:02                         ` Leo Butler
2023-09-21  9:18                           ` Ihor Radchenko
2023-09-21 14:03                             ` Leo Butler
2023-09-22  9:43                               ` Ihor Radchenko
2023-10-02 16:01                                 ` Leo Butler
2023-10-04  8:38                                   ` Ihor Radchenko
2023-10-04 13:07                                     ` Leo Butler
2023-09-02  7:06 ` [BUG] Consider replacing bachload with batch in ob-maxima. [9.6.6 (release_9.6.6 @ /usr/share/emacs/30.0.50/lisp/org/)] Ihor Radchenko
2023-09-02 18:20   ` Leo Butler
2023-09-03  5:25     ` Vladimir Nikishkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87jzsrai3x.fsf@localhost \
    --to=yantar92@posteo.net \
    --cc=Leo.Butler@umanitoba.ca \
    --cc=bzg@gnu.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=for_org-bugs_2023-09-01@lockywolf.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).