Re: ob-octave: improve MATLAB support

[Ihor Radchenko <yantar92@posteo.net>]

John C <john.ciolfi.32@gmail.com> writes:

> Please see the attached patch which makes MATLAB work with org babel
> eval. Here's the commit message:

Thanks a lot for the patch!

The patch introduces major changes. Would it be possible to add some
tests as well? 

> Note, I have sent in the paper work for the FSF copyright assignment.

Let us know when it is done. Also, if there are any problems or delays
in the process, we can try helping.


My initial comments on the patch are below.

> +*** ob-octave: improved MATLAB support

"fixed" maybe :)

> +Improved ob-octave MATLAB (https://www.mathworks.com) code blocks processing.
> +The prior version of ob-octave didn't correctly support MATLAB.  To use MATLAB
> +with org, you need https://github.com/MathWorks/Emacs-MATLAB-Mode.
> +
> +Example ~verbatim~ code block:
> +
> +#+begin_src org
> +,#+begin_src matlab :results verbatim

I am not sure if examples are necessary in the NEWS. Maybe we can simply
mention that :results verbatim, output, and latex are supported now.

As for examples, please put them into
https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-octave-matlab.html
The source code for that page is at https://git.sr.ht/~bzg/worg

> -;; Matlab
> -
> -;; matlab.el required for interactive emacs sessions and matlab-mode
> -;; major mode for source code editing buffer
> -;; https://matlab-emacs.sourceforge.net/
> +;; 1) MATLAB from https://www.mathworks.com

Please remove all the links to mathworks.com - it is FSF policy to not
link to proprietary software pages. We cannot have these links in Org
code.

> +;; 2) https://github.com/mathworks/Emacs-MATLAB-Mode

This one is GPL-licensed and hence fine.

> -(defvar org-babel-default-header-args:matlab '())
> +;; Use the session "*MATLAB*" buffer created by `matlab-shell` for code evaluation.
> +(defvar org-babel-default-header-args:matlab '((:session . "*MATLAB*")))

The new default implies that sessions will be enabled by default. I am
not sure if it is what you intended to do.

> -(defvar org-babel-matlab-with-emacs-link nil
> -  "If non-nil use matlab-shell-run-region for session evaluation.
> -This will use EmacsLink if (matlab-with-emacs-link) evaluates
> -to a non-nil value.")
> -
> -(defvar org-babel-matlab-emacs-link-wrapper-method
> -  "%s
> -if ischar(ans), fid = fopen('%s', 'w'); fprintf(fid, '%%s\\n', ans); fclose(fid);
> -else, save -ascii %s ans
> -end
> -delete('%s')

I understand that these variable no longer make sense after we remove
support of MATLAB Emacs Link. However, I'd prefer to not remove
variables without warning. Instead, please deprecate them.

> +  (when matlabp
> +    ;; When we send multi-line input to `matlab-shell', we'll see the "body"
> +    ;; code lines echoed in the output which is not what one would expect.  To

Maybe you can somehow make use of REMOVE-ECHO in `org-babel-comint-with-output'?

-- 
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>