Hi

Thanks for the feedback. I incorporated some of your suggestions for
org-mode. I'll add the examples to https://git.sr.ht/~bzg/worg once
the changes become part of org-mode.

Regarding (defvar org-babel-default-header-args:matlab '((:session .
"*MATLAB*")), this was done on purpose. We've been using this setting
for a while. Without it, org matlab evaluation is not good. I added a
comment as to why it's there.

I looked into the use of REMOVE-ECHO which ob-octave.el already sets
to t and don't believe this will help us. I think low-level comit
requires the input which is captured by org.

Any guidance on adding a test? To test properly we'll need MATLAB and
https://github.com/mathworks/Emacs-MATLAB-Mode. Note, in
https://github.com/mathworks/Emacs-MATLAB-Mode, I have a test for all
this. Currently Emacs-MATLAB-Mode is advising org to get things
working. When org is updated, Emacs-MATLAB-Mode will automatically not
advise org.

See updated attach patch.

I have yet to hear back from FSF on the copyright assignment. I'm
happy to sign this, so if you can help, that would be good.

Thanks
John

On Sat, Nov 9, 2024 at 4:31 AM Ihor Radchenko <yantar92@posteo.net> wrote:
>
> 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>