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