emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* ob-octave: improve MATLAB support
@ 2024-11-08 19:30 John C
  2024-11-09  9:32 ` Ihor Radchenko
  0 siblings, 1 reply; 7+ messages in thread
From: John C @ 2024-11-08 19:30 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

Hi

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

ob-octave.el: improve MATLAB support

* lisp/ob-octave.el (org-babel-octave-evaluate): Improve MATLAB support
  - Eliminated the code related to MATLAB Emacs Link. This capability was
    removed from MATLAB release R2009a, 15 years ago.
  - Fixed the following type of org block evaluation:
    1) #+begin_src matlab :results verbatim
    2) #+begin_src matlab :results output
    3) #+begin_src matlab :results output latex
    4) #+begin_src matlab :results file graphics
    which aid in writing scientific papers.
  - Minor point, the correct spelling of MATLAB when referencing the product is
    all upper case.

* lisp/ob-matlab.el (header): Update URL for MATLAB

* etc/ORG-NEWS (New functions and changes in function arguments):
  Added entry "ob-octave: improved MATLAB support"

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

Thanks
John

[-- Attachment #2: ob-octave.el.patch --]
[-- Type: text/x-patch, Size: 14725 bytes --]

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 5d421172f..a086ce792 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -162,6 +162,84 @@ bibliography format requires them to be written in title-case.
 
 # This also includes changes in function behavior from Elisp perspective.
 
+*** ob-octave: improved MATLAB support
+
+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
+  a = 2 + 3;
+  ans = magic(a);
+,#+end_src
+
+,#+RESULTS:
+| 17 | 24 |  1 |  8 | 15 |
+| 23 |  5 |  7 | 14 | 16 |
+|  4 |  6 | 13 | 20 | 22 |
+| 10 | 12 | 19 | 21 |  3 |
+| 11 | 18 | 25 |  2 |  9 |
+#+end_src
+
+Example ~output~ code block:
+
+#+begin_src org
+,#+begin_src matlab :results output
+  disp('The results are:')
+  a = [1, 2; 3, 4]
+  b = a * 2
+,#+end_src
+
+,#+RESULTS:
+,#+begin_example
+The results are:
+
+a =
+
+     1     2
+     3     4
+
+b =
+
+     2     4
+     6     8
+,#+end_example
+
+#+end_src
+
+Example ~latex~ code block:
+
+#+begin_src org
+,#+begin_src matlab :results output latex
+  m = [4*pi, 3*pi; 2*pi, pi];
+  result = latex(sym(m));
+  disp(result)
+,#+end_src
+
+,#+RESULTS:
+,#+begin_export latex
+\left(\begin{array}{cc} 4\,\pi  & 3\,\pi \\ 2\,\pi  & \pi  \end{array}\right)
+,#+end_export
+#+end_src
+
+Example ~graphics~ code block:
+
+#+begin_src org
+,#+header: :file sinewave.png
+,#+begin_src matlab :results file graphics
+  t = [0 : 0.1 : 2*pi];
+  y = sin(t);
+  plot(t, y);
+  set(gcf, 'PaperUnits', 'inches', 'PaperPosition', [0 0 4 3]) % Set the size to 4" x 3"
+,#+end_src
+
+,#+RESULTS:
+[[file:sinewave.png]]
+#+end_src
+
 *** ob-sqlite: Added ability to open a database in readonly mode
 
 Added option :readonly to ob-sqlite.
diff --git a/lisp/ob-matlab.el b/lisp/ob-matlab.el
index de8deadbe..bea40482b 100644
--- a/lisp/ob-matlab.el
+++ b/lisp/ob-matlab.el
@@ -28,11 +28,10 @@
 
 ;;; Requirements:
 
-;; 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
+;; 2) https://github.com/mathworks/Emacs-MATLAB-Mode
+;;    For matlab-shell to run MATLAB within Emacs and matlab-mode
+;;    major mode for source code editing buffer
 
 ;;; Code:
 
diff --git a/lisp/ob-octave.el b/lisp/ob-octave.el
index 005990f20..39a20b57f 100644
--- a/lisp/ob-octave.el
+++ b/lisp/ob-octave.el
@@ -1,4 +1,4 @@
-;;; ob-octave.el --- Babel Functions for Octave and Matlab -*- lexical-binding: t; -*-
+;;; ob-octave.el --- Babel Functions for Octave and MATLAB -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 2010-2024 Free Software Foundation, Inc.
 
@@ -30,6 +30,8 @@
 
 ;;; Code:
 
+(require 'cl-seq)
+
 (require 'org-macs)
 (org-assert-version)
 
@@ -39,7 +41,9 @@
 (declare-function matlab-shell "ext:matlab-mode")
 (declare-function matlab-shell-run-region "ext:matlab-mode")
 
-(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*")))
+
 (defvar org-babel-default-header-args:octave '())
 
 (defvar org-babel-matlab-shell-command "matlab -nosplash"
@@ -47,18 +51,36 @@
 (defvar org-babel-octave-shell-command "octave -q"
   "Shell command to run octave as an external process.")
 
-(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')
+(defvar org-babel-matlab-print "print(\"-dpng\", %S);\nans=%S;"
+  ;; MATLAB command-function duality requires that the file name be specified
+  ;; without quotes. Using: print -dpng "file.png", would produce a file with
+  ;; the quotes in the file name on disk. Therefore, use the functional form
+  ;; to handle files with spaces, print("-dpng", "file.png").
+  ;; Example:
+  ;;    #+begin_src matlab :file "sine wave.png" :results file graphics
+  ;;      t = [0 : 0.1 : 2*pi];
+  ;;      y = sin(t);
+  ;;      plot(t, y);
+  ;;      set(gcf, 'PaperUnits', 'inches', 'PaperPosition', [0 0 4 3]) % Set the size to 4" x 3"
+  ;;    #+end_src
+  ;;
+  ;;    #+RESULTS:
+  ;;    [[file:sine wave.png]]
+  "MATLAB format specifier to print current figure to a file.")
+
+(defvar org-babel-octave-print "print -dpng %S\nans=%S"
+  "Octave format specifier to print current figure to a file.")
+
+(defvar org-babel-matlab-wrapper-method
+  (concat "\
+cd('%s');
+%s
+if ~exist('ans', 'var') ans = ''; end; \
+writematrix(ans, '%s', 'Delimiter', 'tab');
 ")
+  "Format specifier used when evaluating MATLAB code blocks.
+Arguments are the `default-directory', the MATLAB code, and a result file.txt.")
+
 (defvar org-babel-octave-wrapper-method
   "%s
 if ischar(ans), fid = fopen('%s', 'w'); fdisp(fid, ans); fclose(fid);
@@ -92,7 +114,10 @@ When MATLABP is non-nil, execute Matlab.  Otherwise, execute Octave."
 				 (list
 				  "set (0, \"defaultfigurevisible\", \"off\");"
 				  full-body
-				  (format "print -dpng %S\nans=%S" gfx-file gfx-file))
+				  (format (if matlabp
+                                              org-babel-matlab-print
+                                            org-babel-octave-print)
+                                          gfx-file gfx-file))
 				 "\n")
 		    full-body)
 		  result-type matlabp)))
@@ -153,6 +178,23 @@ If there is not a current inferior-process-buffer in SESSION then
 create.  Return the initialized session.  PARAMS are src block parameters."
   (org-babel-octave-initiate-session session params 'matlab))
 
+(defun org-babel-matlab-shell ()
+  "Start and/or wait for MATLAB shell."
+  (require 'matlab-shell) ;; make `matlab-shell-busy-checker' available
+  (cond
+   ((fboundp 'matlab-shell-busy-checker)
+    ;; Start the shell if needed.  `matlab-shell' will reuse existing if already running.
+    (matlab-shell)
+    ;; If we just started the matlab-shell, wait for the prompt.  If we do not
+    ;; wait, then the startup messages will show up in the evaluation results.
+    (matlab-shell-busy-checker 'wait-for-prompt))
+   (t
+    (message (concat "You version of matlab-mode is old.\n"
+                     "Please update, see https://github.com/mathworks/Emacs-MATLAB-Mode\n"
+                     "Updating will eliminate unexpected output in your results\n"))
+    (sit-for 3)
+    (matlab-shell))))
+
 (defun org-babel-octave-initiate-session (&optional session _params matlabp)
   "Create an octave inferior process buffer.
 If there is not a current inferior-process-buffer in SESSION then
@@ -165,9 +207,15 @@ Octave session, unless MATLABP is non-nil."
   (unless (string= session "none")
     (let ((session (or session
 		       (if matlabp "*Inferior Matlab*" "*Inferior Octave*"))))
-      (if (org-babel-comint-buffer-livep session) session
+      (if (org-babel-comint-buffer-livep session)
+          (progn
+            (when (and matlabp (fboundp 'matlab-shell-busy-checker))
+              ;; Can't evaluate if the matlab-shell is currently running code
+              (matlab-shell-busy-checker 'error-if-busy))
+            session)
 	(save-window-excursion
-	  (if matlabp (unless org-babel-matlab-with-emacs-link (matlab-shell))
+	  (if matlabp
+              (org-babel-matlab-shell)
 	    (run-octave))
 	  (rename-buffer (if (bufferp session) (buffer-name session)
 			   (if (stringp session) session (buffer-name))))
@@ -183,66 +231,107 @@ value of the last statement in BODY, as elisp."
       (org-babel-octave-evaluate-session session body result-type matlabp)
     (org-babel-octave-evaluate-external-process body result-type matlabp)))
 
+(defun org-babel-octave-wrapper-tmp-file (matlabp)
+  "Return a local tmp file with name adjusted for MATLABP."
+  (if matlabp
+      ;; writematrix requires a file ending with '.txt'
+      (org-babel-temp-file "matlab-" ".txt")
+    (org-babel-temp-file "octave-")))
+
+(defun org-babel-octave-get-code-to-eval (body tmp-file matlabp)
+  "Format BODY of the code block for evaluation saving results to TMP-FILE.
+If MATLABP, format for MATLAB, else format for Octave."
+    (if matlabp
+        (format org-babel-matlab-wrapper-method default-directory body tmp-file)
+      (format org-babel-octave-wrapper-method body tmp-file tmp-file)))
+
 (defun org-babel-octave-evaluate-external-process (body result-type matlabp)
-  "Evaluate BODY in an external Octave or Matalab process.
+  "Evaluate BODY in an external Octave or MATLAB process.
 Process the result as RESULT-TYPE.  Use Octave, unless MATLABP is non-nil."
   (let ((cmd (if matlabp
 		 org-babel-matlab-shell-command
 	       org-babel-octave-shell-command)))
     (pcase result-type
       (`output (org-babel-eval cmd body))
-      (`value (let ((tmp-file (org-babel-temp-file "octave-")))
+      (`value (let ((tmp-file (org-babel-process-file-name
+                               (org-babel-octave-wrapper-tmp-file matlabp)
+                               'noquote)))
 	        (org-babel-eval
 		 cmd
-		 (format org-babel-octave-wrapper-method body
-			 (org-babel-process-file-name tmp-file 'noquote)
-			 (org-babel-process-file-name tmp-file 'noquote)))
+                 (org-babel-octave-get-code-to-eval body tmp-file matlabp))
 	        (org-babel-octave-import-elisp-from-file tmp-file))))))
 
+(defun org-babel-body-for-output (body matlabp)
+  "If MATLABP, fixup BODY for MATLAB output result-type."
+  (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
+    ;; remove these unwanted lines, we append a comment "%-<org-eval>" to each
+    ;; line in the body MATLAB code.  After we collect the results from
+    ;; evaluation, we leverage the "%-<org-eval>" to remove the unwanted lines.
+    ;; Example of desired behavior:
+    ;;     #+begin_src matlab :results output
+    ;;       disp('The results are:')
+    ;;       a = [1, 2; 3, 4]
+    ;;       b = a * 2
+    ;;     #+end_src
+    ;;
+    ;;     #+RESULTS:
+    ;;     #+begin_example
+    ;;     The results are:
+    ;;
+    ;;     a =
+    ;;
+    ;;          1     2
+    ;;          3     4
+    ;;
+    ;;     b =
+    ;;
+    ;;          2     4
+    ;;          6     8
+    ;;     #+end_example
+
+    (setq body (replace-regexp-in-string "\n" " %-<org-eval>-\n" body))
+    (when (not (string-match "\n\\'" body))
+      (setq body (concat body " %-<org-eval>-"))))
+  body)
+
+(defun org-babel-fix-up-output (results)
+  "Fix up RESULTS for output result-type."
+  ;; When we send multi-line input to `matlab-shell', we'll see the "body" code
+  ;; lines echoed in the output. Therefore, leverage the "%-<org-eval>" to
+  ;; remove the unnecessary lines.
+  (let ((fixed-results (replace-regexp-in-string "^[^\n]*%-<org-eval>-\n" ""
+                                                 results)))
+    ;; Remove unnecessary starting blank line caused by stripping %-<org-eval>
+    (replace-regexp-in-string "\\`[[:space:]\r\n]+" "" fixed-results)))
+
 (defun org-babel-octave-evaluate-session
     (session body result-type &optional matlabp)
   "Evaluate BODY in SESSION."
-  (let* ((tmp-file (org-babel-temp-file (if matlabp "matlab-" "octave-")))
-	 (wait-file (org-babel-temp-file "matlab-emacs-link-wait-signal-"))
+  (let* ((tmp-file (org-babel-octave-wrapper-tmp-file matlabp))
 	 (full-body
 	  (pcase result-type
 	    (`output
 	     (mapconcat
 	      #'org-babel-chomp
-	      (list body org-babel-octave-eoe-indicator) "\n"))
+	      (list (org-babel-body-for-output body matlabp)
+                    org-babel-octave-eoe-indicator)
+              "\n"))
 	    (`value
-	     (if (and matlabp org-babel-matlab-with-emacs-link)
-		 (concat
-		  (format org-babel-matlab-emacs-link-wrapper-method
-			  body
-			  (org-babel-process-file-name tmp-file 'noquote)
-			  (org-babel-process-file-name tmp-file 'noquote) wait-file) "\n")
-	       (mapconcat
-		#'org-babel-chomp
-		(list (format org-babel-octave-wrapper-method
-			      body
-			      (org-babel-process-file-name tmp-file 'noquote)
-			      (org-babel-process-file-name tmp-file 'noquote))
-		      org-babel-octave-eoe-indicator) "\n")))))
-	 (raw (if (and matlabp org-babel-matlab-with-emacs-link)
-		  (save-window-excursion
-		    (with-temp-buffer
-		      (insert full-body)
-		      (write-region "" 'ignored wait-file nil nil nil 'excl)
-		      (matlab-shell-run-region (point-min) (point-max))
-		      (message "Waiting for Matlab Emacs Link")
-		      (while (file-exists-p wait-file) (sit-for 0.01))
-		      "")) ;; matlab-shell-run-region doesn't seem to
-		;; make *matlab* buffer contents easily
-		;; available, so :results output currently
-		;; won't work
-		(org-babel-comint-with-output
-		    (session
-		     (if matlabp
-			 org-babel-octave-eoe-indicator
-		       org-babel-octave-eoe-output)
-		     t full-body)
-		  (insert full-body) (comint-send-input nil t))))
+	     (mapconcat
+	      #'org-babel-chomp
+	      (list (org-babel-octave-get-code-to-eval body tmp-file matlabp)
+		    org-babel-octave-eoe-indicator)
+              "\n"))))
+	 (raw-results
+	  (org-babel-comint-with-output
+	      (session
+	       (if matlabp
+		   org-babel-octave-eoe-indicator
+		 org-babel-octave-eoe-output)
+	       t full-body)
+	    (insert full-body) (comint-send-input nil t)))
 	 results)
     (pcase result-type
       (`value
@@ -250,12 +339,13 @@ Process the result as RESULT-TYPE.  Use Octave, unless MATLABP is non-nil."
       (`output
        (setq results
 	     (if matlabp
-		 (cdr (reverse (delete "" (mapcar #'org-strip-quotes
-					          (mapcar #'org-trim raw)))))
+                 (cdr (reverse (delete "" (mapcar #'org-strip-quotes
+					          (mapcar #'org-trim raw-results)))))
 	       (cdr (member org-babel-octave-eoe-output
 			    (reverse (mapcar #'org-strip-quotes
-					     (mapcar #'org-trim raw)))))))
-       (mapconcat #'identity (reverse results) "\n")))))
+					     (mapcar #'org-trim raw-results)))))))
+       (org-babel-fix-up-output
+        (mapconcat #'identity (reverse results) "\n"))))))
 
 (defun org-babel-octave-import-elisp-from-file (file-name)
   "Import data from FILE-NAME.

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: ob-octave: improve MATLAB support
  2024-11-08 19:30 ob-octave: improve MATLAB support John C
@ 2024-11-09  9:32 ` Ihor Radchenko
  2024-11-13 19:10   ` John C
  0 siblings, 1 reply; 7+ messages in thread
From: Ihor Radchenko @ 2024-11-09  9:32 UTC (permalink / raw)
  To: John C; +Cc: emacs-orgmode

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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ob-octave: improve MATLAB support
  2024-11-09  9:32 ` Ihor Radchenko
@ 2024-11-13 19:10   ` John C
  2024-11-15 13:29     ` John C
  2024-11-23 15:57     ` Ihor Radchenko
  0 siblings, 2 replies; 7+ messages in thread
From: John C @ 2024-11-13 19:10 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 4576 bytes --]

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>

[-- Attachment #2: ob-octave.el.patch --]
[-- Type: text/x-patch, Size: 15466 bytes --]

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 5d421172f..5513c172d 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -162,6 +162,13 @@ bibliography format requires them to be written in title-case.
 
 # This also includes changes in function behavior from Elisp perspective.
 
+*** ob-octave: fixed MATLAB support
+
+Fixed 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. Now ~#+begin_src matlab~ code blocks with ~:results
+verbatim~, ~:results output~, ~:results output latex~, or ~:results file graphics~ are supported.
+
 *** ob-sqlite: Added ability to open a database in readonly mode
 
 Added option :readonly to ob-sqlite.
diff --git a/lisp/ob-matlab.el b/lisp/ob-matlab.el
index de8deadbe..083dcdec3 100644
--- a/lisp/ob-matlab.el
+++ b/lisp/ob-matlab.el
@@ -28,11 +28,10 @@
 
 ;;; Requirements:
 
-;; 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
+;; 2) https://github.com/mathworks/Emacs-MATLAB-Mode
+;;    For matlab-shell to run MATLAB within Emacs and matlab-mode
+;;    major mode for source code editing buffer
 
 ;;; Code:
 
diff --git a/lisp/ob-octave.el b/lisp/ob-octave.el
index 005990f20..5ac1de1e4 100644
--- a/lisp/ob-octave.el
+++ b/lisp/ob-octave.el
@@ -1,4 +1,4 @@
-;;; ob-octave.el --- Babel Functions for Octave and Matlab -*- lexical-binding: t; -*-
+;;; ob-octave.el --- Babel Functions for Octave and MATLAB -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 2010-2024 Free Software Foundation, Inc.
 
@@ -30,6 +30,8 @@
 
 ;;; Code:
 
+(require 'cl-seq)
+
 (require 'org-macs)
 (org-assert-version)
 
@@ -39,7 +41,52 @@
 (declare-function matlab-shell "ext:matlab-mode")
 (declare-function matlab-shell-run-region "ext:matlab-mode")
 
-(defvar org-babel-default-header-args:matlab '())
+;; With `org-babel-default-header-args:matlab' set to
+;;  '((:session . "*MATLAB*")))
+;; each matlab code block evaluation will reuse the "*MATLAB*" buffer
+;; created by `matlab-shell' for code evaluation.  The benefit of this
+;; is that evaluation is very fast for evaluations after the first
+;; evaluation. The first evaluation may be slower because it can take
+;; a long time to start MATLAB.  Reusing the session buffer means
+;; state is maintained between evaluations.  To avoid reuse of state,
+;; you can clear the MATLAB workspace.  This setting aligns with
+;; typical MATLAB use, where MATLAB is started and used for a long
+;; period of period of time.  Another benefit of this setting is that
+;; you can see the history of the evaluations in the "*MATLAB*"
+;; command window buffer.  For example:
+;;
+;;  #+begin_src matlab :results output
+;;    a = 123
+;;  #+end_src
+;;
+;;  #+RESULTS:
+;;  : a =
+;;  :
+;;  :    123
+;;
+;;  #+begin_src matlab :results output
+;;    b = a * 2
+;;  #+end_src
+;;
+;;  #+RESULTS:
+;;  : b =
+;;  :
+;;  :    246
+;;
+;;  #+begin_src matlab :results output
+;;    clear
+;;    c = b * 2
+;;  #+end_src
+;;
+;;  #+RESULTS:
+;;  : Unrecognized function or variable 'b'.
+;;
+;; If you want a new session each time you evaluate a MATLAB code block,
+;;   (setq 'org-babel-default-header-args:matlab '())
+;; However, this will make evaluations slower and is not typically how
+;; people use MATLAB.
+(defvar org-babel-default-header-args:matlab '((:session . "*MATLAB*")))
+
 (defvar org-babel-default-header-args:octave '())
 
 (defvar org-babel-matlab-shell-command "matlab -nosplash"
@@ -47,18 +94,42 @@
 (defvar org-babel-octave-shell-command "octave -q"
   "Shell command to run octave as an external process.")
 
-(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')
+(make-obsolete-variable 'org-babel-matlab-with-emacs-link
+                        "MATLAB removed EmacsLink in R2009a." "2009")
+
+(make-obsolete-variable 'org-babel-matlab-emacs-link-wrapper-method
+                        "MATLAB removed EmacsLink in R2009a." "2009")
+
+(defvar org-babel-matlab-print "print(\"-dpng\", %S);\nans=%S;"
+  ;; MATLAB command-function duality requires that the file name be specified
+  ;; without quotes. Using: print -dpng "file.png", would produce a file with
+  ;; the quotes in the file name on disk. Therefore, use the functional form
+  ;; to handle files with spaces, print("-dpng", "file.png").
+  ;; Example:
+  ;;    #+begin_src matlab :file "sine wave.png" :results file graphics
+  ;;      t = [0 : 0.1 : 2*pi];
+  ;;      y = sin(t);
+  ;;      plot(t, y);
+  ;;      set(gcf, 'PaperUnits', 'inches', 'PaperPosition', [0 0 4 3]) % Set the size to 4" x 3"
+  ;;    #+end_src
+  ;;
+  ;;    #+RESULTS:
+  ;;    [[file:sine wave.png]]
+  "MATLAB format specifier to print current figure to a file.")
+
+(defvar org-babel-octave-print "print -dpng %S\nans=%S"
+  "Octave format specifier to print current figure to a file.")
+
+(defvar org-babel-matlab-wrapper-method
+  (concat "\
+cd('%s');
+%s
+if ~exist('ans', 'var') ans = ''; end; \
+writematrix(ans, '%s', 'Delimiter', 'tab');
 ")
+  "Format specifier used when evaluating MATLAB code blocks.
+Arguments are the `default-directory', the MATLAB code, and a result file.txt.")
+
 (defvar org-babel-octave-wrapper-method
   "%s
 if ischar(ans), fid = fopen('%s', 'w'); fdisp(fid, ans); fclose(fid);
@@ -92,7 +163,10 @@ When MATLABP is non-nil, execute Matlab.  Otherwise, execute Octave."
 				 (list
 				  "set (0, \"defaultfigurevisible\", \"off\");"
 				  full-body
-				  (format "print -dpng %S\nans=%S" gfx-file gfx-file))
+				  (format (if matlabp
+                                              org-babel-matlab-print
+                                            org-babel-octave-print)
+                                          gfx-file gfx-file))
 				 "\n")
 		    full-body)
 		  result-type matlabp)))
@@ -153,6 +227,23 @@ If there is not a current inferior-process-buffer in SESSION then
 create.  Return the initialized session.  PARAMS are src block parameters."
   (org-babel-octave-initiate-session session params 'matlab))
 
+(defun org-babel-matlab-shell ()
+  "Start and/or wait for MATLAB shell."
+  (require 'matlab-shell) ;; make `matlab-shell-busy-checker' available
+  (cond
+   ((fboundp 'matlab-shell-busy-checker)
+    ;; Start the shell if needed.  `matlab-shell' will reuse existing if already running.
+    (matlab-shell)
+    ;; If we just started the matlab-shell, wait for the prompt.  If we do not
+    ;; wait, then the startup messages will show up in the evaluation results.
+    (matlab-shell-busy-checker 'wait-for-prompt))
+   (t
+    (message (concat "You version of matlab-mode is old.\n"
+                     "Please update, see https://github.com/mathworks/Emacs-MATLAB-Mode\n"
+                     "Updating will eliminate unexpected output in your results\n"))
+    (sit-for 3)
+    (matlab-shell))))
+
 (defun org-babel-octave-initiate-session (&optional session _params matlabp)
   "Create an octave inferior process buffer.
 If there is not a current inferior-process-buffer in SESSION then
@@ -165,9 +256,15 @@ Octave session, unless MATLABP is non-nil."
   (unless (string= session "none")
     (let ((session (or session
 		       (if matlabp "*Inferior Matlab*" "*Inferior Octave*"))))
-      (if (org-babel-comint-buffer-livep session) session
+      (if (org-babel-comint-buffer-livep session)
+          (progn
+            (when (and matlabp (fboundp 'matlab-shell-busy-checker))
+              ;; Can't evaluate if the matlab-shell is currently running code
+              (matlab-shell-busy-checker 'error-if-busy))
+            session)
 	(save-window-excursion
-	  (if matlabp (unless org-babel-matlab-with-emacs-link (matlab-shell))
+	  (if matlabp
+              (org-babel-matlab-shell)
 	    (run-octave))
 	  (rename-buffer (if (bufferp session) (buffer-name session)
 			   (if (stringp session) session (buffer-name))))
@@ -183,66 +280,111 @@ value of the last statement in BODY, as elisp."
       (org-babel-octave-evaluate-session session body result-type matlabp)
     (org-babel-octave-evaluate-external-process body result-type matlabp)))
 
+(defun org-babel-octave-wrapper-tmp-file (matlabp)
+  "Return a local tmp file with name adjusted for MATLABP."
+  (if matlabp
+      ;; writematrix requires a file ending with '.txt'
+      (org-babel-temp-file "matlab-" ".txt")
+    (org-babel-temp-file "octave-")))
+
+(defun org-babel-octave-get-code-to-eval (body tmp-file matlabp)
+  "Format BODY of the code block for evaluation saving results to TMP-FILE.
+If MATLABP, format for MATLAB, else format for Octave."
+    (if matlabp
+        (format org-babel-matlab-wrapper-method default-directory body tmp-file)
+      (format org-babel-octave-wrapper-method body tmp-file tmp-file)))
+
 (defun org-babel-octave-evaluate-external-process (body result-type matlabp)
-  "Evaluate BODY in an external Octave or Matalab process.
+  "Evaluate BODY in an external Octave or MATLAB process.
 Process the result as RESULT-TYPE.  Use Octave, unless MATLABP is non-nil."
   (let ((cmd (if matlabp
 		 org-babel-matlab-shell-command
 	       org-babel-octave-shell-command)))
     (pcase result-type
       (`output (org-babel-eval cmd body))
-      (`value (let ((tmp-file (org-babel-temp-file "octave-")))
+      (`value (let ((tmp-file (org-babel-process-file-name
+                               (org-babel-octave-wrapper-tmp-file matlabp)
+                               'noquote)))
 	        (org-babel-eval
 		 cmd
-		 (format org-babel-octave-wrapper-method body
-			 (org-babel-process-file-name tmp-file 'noquote)
-			 (org-babel-process-file-name tmp-file 'noquote)))
+                 (org-babel-octave-get-code-to-eval body tmp-file matlabp))
 	        (org-babel-octave-import-elisp-from-file tmp-file))))))
 
+(defun org-babel-body-for-output (body matlabp)
+  "If MATLABP, fixup BODY for MATLAB output result-type."
+  (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
+    ;; remove these unwanted lines, we append a comment "%-<org-eval>" to each
+    ;; line in the body MATLAB code.  After we collect the results from
+    ;; evaluation, we leverage the "%-<org-eval>" to remove the unwanted lines.
+    ;; Example of desired behavior:
+    ;;     #+begin_src matlab :results output
+    ;;       disp('The results are:')
+    ;;       a = [1, 2; 3, 4]
+    ;;       b = a * 2
+    ;;     #+end_src
+    ;;
+    ;;     #+RESULTS:
+    ;;     #+begin_example
+    ;;     The results are:
+    ;;
+    ;;     a =
+    ;;
+    ;;          1     2
+    ;;          3     4
+    ;;
+    ;;     b =
+    ;;
+    ;;          2     4
+    ;;          6     8
+    ;;     #+end_example
+
+    (setq body (replace-regexp-in-string "\n" " %-<org-eval>-\n" body))
+    (when (not (string-match "\n\\'" body))
+      (setq body (concat body " %-<org-eval>-"))))
+  body)
+
+(defun org-babel-fix-up-output (results matlabp)
+  "Fix up RESULTS for output result-type when MATLABP."
+  ;; When we send multi-line input to `matlab-shell', we'll see the "body" code
+  ;; lines echoed in the output.  Therefore, leverage the "%-<org-eval>" to
+  ;; remove the unnecessary lines.
+  (if matlabp
+      (let ((fixed-results (replace-regexp-in-string "^[^\n]*%-<org-eval>-\n" ""
+                                                     results)))
+        ;; Remove unnecessary starting blank line caused by stripping %-<org-eval>
+        (setq fixed-results (replace-regexp-in-string "\\`[[:space:]\r\n]+" "" fixed-results))
+        ;; matlab-shell wraps errors in <ERRORTXT> and </ERRORTXT> so remove those
+        (replace-regexp-in-string "</?ERRORTXT>[\r\n]" "" fixed-results))
+    results))
+
 (defun org-babel-octave-evaluate-session
     (session body result-type &optional matlabp)
   "Evaluate BODY in SESSION."
-  (let* ((tmp-file (org-babel-temp-file (if matlabp "matlab-" "octave-")))
-	 (wait-file (org-babel-temp-file "matlab-emacs-link-wait-signal-"))
+  (let* ((tmp-file (org-babel-octave-wrapper-tmp-file matlabp))
 	 (full-body
 	  (pcase result-type
 	    (`output
 	     (mapconcat
 	      #'org-babel-chomp
-	      (list body org-babel-octave-eoe-indicator) "\n"))
+	      (list (org-babel-body-for-output body matlabp)
+                    org-babel-octave-eoe-indicator)
+              "\n"))
 	    (`value
-	     (if (and matlabp org-babel-matlab-with-emacs-link)
-		 (concat
-		  (format org-babel-matlab-emacs-link-wrapper-method
-			  body
-			  (org-babel-process-file-name tmp-file 'noquote)
-			  (org-babel-process-file-name tmp-file 'noquote) wait-file) "\n")
-	       (mapconcat
-		#'org-babel-chomp
-		(list (format org-babel-octave-wrapper-method
-			      body
-			      (org-babel-process-file-name tmp-file 'noquote)
-			      (org-babel-process-file-name tmp-file 'noquote))
-		      org-babel-octave-eoe-indicator) "\n")))))
-	 (raw (if (and matlabp org-babel-matlab-with-emacs-link)
-		  (save-window-excursion
-		    (with-temp-buffer
-		      (insert full-body)
-		      (write-region "" 'ignored wait-file nil nil nil 'excl)
-		      (matlab-shell-run-region (point-min) (point-max))
-		      (message "Waiting for Matlab Emacs Link")
-		      (while (file-exists-p wait-file) (sit-for 0.01))
-		      "")) ;; matlab-shell-run-region doesn't seem to
-		;; make *matlab* buffer contents easily
-		;; available, so :results output currently
-		;; won't work
-		(org-babel-comint-with-output
-		    (session
-		     (if matlabp
-			 org-babel-octave-eoe-indicator
-		       org-babel-octave-eoe-output)
-		     t full-body)
-		  (insert full-body) (comint-send-input nil t))))
+	     (mapconcat
+	      #'org-babel-chomp
+	      (list (org-babel-octave-get-code-to-eval body tmp-file matlabp)
+		    org-babel-octave-eoe-indicator)
+              "\n"))))
+	 (raw-results
+	  (org-babel-comint-with-output
+	      (session
+	       (if matlabp
+		   org-babel-octave-eoe-indicator
+		 org-babel-octave-eoe-output)
+	       t full-body)
+	    (insert full-body) (comint-send-input nil t)))
 	 results)
     (pcase result-type
       (`value
@@ -250,12 +392,13 @@ Process the result as RESULT-TYPE.  Use Octave, unless MATLABP is non-nil."
       (`output
        (setq results
 	     (if matlabp
-		 (cdr (reverse (delete "" (mapcar #'org-strip-quotes
-					          (mapcar #'org-trim raw)))))
+                 (cdr (reverse (delete "" (mapcar #'org-strip-quotes
+					          (mapcar #'org-trim raw-results)))))
 	       (cdr (member org-babel-octave-eoe-output
 			    (reverse (mapcar #'org-strip-quotes
-					     (mapcar #'org-trim raw)))))))
-       (mapconcat #'identity (reverse results) "\n")))))
+					     (mapcar #'org-trim raw-results)))))))
+       (org-babel-fix-up-output
+        (mapconcat #'identity (reverse results) "\n") matlabp)))))
 
 (defun org-babel-octave-import-elisp-from-file (file-name)
   "Import data from FILE-NAME.

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: ob-octave: improve MATLAB support
  2024-11-13 19:10   ` John C
@ 2024-11-15 13:29     ` John C
  2024-11-23 15:57     ` Ihor Radchenko
  1 sibling, 0 replies; 7+ messages in thread
From: John C @ 2024-11-15 13:29 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

I received the FSF form to assign and will be sending that in soon.

On Wed, Nov 13, 2024 at 2:10 PM John C <john.ciolfi.32@gmail.com> wrote:
>
> 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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ob-octave: improve MATLAB support
  2024-11-13 19:10   ` John C
  2024-11-15 13:29     ` John C
@ 2024-11-23 15:57     ` Ihor Radchenko
       [not found]       ` <87o712lv3t.fsf@localhost>
  1 sibling, 1 reply; 7+ messages in thread
From: Ihor Radchenko @ 2024-11-23 15:57 UTC (permalink / raw)
  To: John C; +Cc: emacs-orgmode

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

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

Thanks for the update!

> 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 can see the usefulness of using session by default for matlab, but it
is still going to be a breaking change. As a minimum, we should announce
it in the NEWS.

You may (1) split your news entry into "fix" part and ":results
handling" ; (2) add notice that matlab blocks will use session by default

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

May you please elaborate what exactly is not working when using
REMOVE-ECHO? Note that you normally need to pass the body alongside with
REMOVE-ECHO argument and `org-babel-comint-with-output' should remove
everything up to FULL-BODY before returning the output.

The problem you are trying to solve _should_ be solved by
`org-babel-comit-with-output'. If it is not, it is a bug that we should
fix.

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

For tests, you can refer to testing/lisp/test-ob-R.el tests. They also
rely upon external package (ESS) and on external R executable.

Ideally, we want to reuse tests from testing/lisp/test-ob-octave.el

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ob-octave: improve MATLAB support
       [not found]       ` <87o712lv3t.fsf@localhost>
@ 2024-12-29  3:04         ` John C
  2024-12-29  7:42           ` Ihor Radchenko
  0 siblings, 1 reply; 7+ messages in thread
From: John C @ 2024-12-29  3:04 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 9163 bytes --]

I accidentally did "reply" instead of "reply all". Re-adding the
e-mail between Ihor and I.

Regarding the suggestion to filter line-by-line. I think I understand that,
but it will require that we match input lines to echo'd input. However,
it's possible that output lines match input and are not from echo'd input.
In addition, there's other stripping (filtering) that is needed. For
example, removal of the error indicators (<ERRORTXT>). I like the idea of
enhancing org-babel-comint-with-output to strip content based on regular
expressions and have implemented that.

See attached org-matlab.patch which addresses all feedback. Here's the
commit info.

ob-matlab.el: improve MATLAB support

* lisp/ob-matlab.el (header): Update URL for MATLAB

* lisp/ob-octave.el (org-babel-octave-evaluate): Fixed MATLAB support
  - Deprecate variables related to MATLAB Emacs Link and removed the code.
    Emacs Link capability was removed from MATLAB release R2009a, 15 years
ago.
  - Fixed the following type of org block evaluation:
    1) #+begin_src matlab :results verbatim
    2) #+begin_src matlab :results output
    3) #+begin_src matlab :results output latex
    4) #+begin_src matlab :results file graphics
    which aid in writing scientific papers.
  - Minor point, the correct spelling of MATLAB when referencing the
product is
    all upper case.

* ob-comint.el: enhanced org-babel-comint-with-output
  - The META argument of org-babel-comint-with-output now supports an
optional
    STRIP-REGEXPS which can be used to remove content from the returned
output.

* etc/ORG-NEWS (New functions and changes in function arguments):
  Added entry "ob-octave: improved MATLAB support"

Thanks
John

-----
*Ihor Radchenko *<yantar92@posteo.net> Sun, Dec 22, 2024 at 11:24 PM

To: John C <john.ciolfi.32@gmail.com>
Cc: emacs-orgmode@gnu.org
Hi John,

It has been a month since the last message in this thread.
Do you need any help working on the patch?

--
Ihor Radchenko // yantar92,
Org mode maintainer,
[Quoted text hidden]
------------------------------
*John C *<john.ciolfi.32@gmail.com> Wed, Dec 25, 2024 at 8:26 AM

To: Ihor Radchenko <yantar92@posteo.net>
My apologies - I’ve been too busy. I'll update the NEWS entry per your
suggestion once we resolve the "%-<org-eval>" comment issue you flagged.

Consider this MATLAB code block and expected results.

    #+begin_src matlab :exports both :results output latex
      m = [4*pi, 3*pi; 2*pi, pi];
      result = latex(sym(m));
      disp(result)
    #+end_src

    #+RESULTS:
    #+begin_export latex
    \left(\begin{array}{cc} 4\,\pi  & 3\,\pi \\ 2\,\pi  & \pi
 \end{array}\right)
    #+end_export

If we remove the proposed "%-<org-eval>" handling from ob-octave.el, we get
the incorrect result:

    #+RESULTS:
    #+begin_export latex
    m = [4*pi, 3*pi; 2*pi, pi];
    result = latex(sym(m));
    disp(result)
    \left(\begin{array}{cc} 4\,\pi  & 3\,\pi \\ 2\,\pi  & \pi
 \end{array}\right)
    #+end_export

In this case, the *MATLAB* buffer contains:

    >> m = [4*pi, 3*pi; 2*pi, pi];
    result = latex(sym(m));
    disp(result)
    'org_babel_eoe'
    m = [4*pi, 3*pi; 2*pi, pi];
    >> result = latex(sym(m));
    >> disp(result)
    \left(\begin{array}{cc} 4\,\pi  & 3\,\pi \\ 2\,\pi  & \pi
 \end{array}\right)
    >> 'org_babel_eoe'

    ans =

        'org_babel_eoe'

    >>

I suspect that the issue is that the MATLAB echo's all commands and that
there's noting wrong with comint. For example, matlab-shell buffer,
*MATLAB*, the current buffer and at the MATLAB promot, ">>",

    M-: RET (comint-send-string (get-buffer-process (current-buffer))
"pwd\n") RET

Results in:

    #+begin_example
    >> pwd

    ans =

        '/home/ciolfi/tmp'
    #+end_example

Therefore, the adding "%-<org-eval>" MATLAB comments to the code being
evaluated, then stripping the echo'd lines containing the "%-<org-eval>" is
probably the best option.

Thanks
John
[Quoted text hidden]
------------------------------
*Ihor Radchenko *<yantar92@posteo.net> Thu, Dec 26, 2024 at 1:02 AM

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

> My apologies - I’ve been too busy. I'll update the NEWS entry per your
> suggestion once we resolve the "%-<org-eval>" comment issue you flagged.
>
> Consider this MATLAB code block and expected results.
>
>     #+begin_src matlab :exports both :results output latex
>       m = [4*pi, 3*pi; 2*pi, pi];
>       result = latex(sym(m));
>       disp(result)
>     #+end_src
> ...
> In this case, the *MATLAB* buffer contains:
>
>     >> m = [4*pi, 3*pi; 2*pi, pi];
>     result = latex(sym(m));
>     disp(result)
>     'org_babel_eoe'
>     m = [4*pi, 3*pi; 2*pi, pi];
>     >> result = latex(sym(m));
>     >> disp(result)
>     \left(\begin{array}{cc} 4\,\pi  & 3\,\pi \\ 2\,\pi  & \pi
>  \end{array}\right)
>     >> 'org_babel_eoe'
>
>     ans =
>
>         'org_babel_eoe'
>
>     >>

So, MATLAB does not echo the full code, but rather does it line-by-line.
This is not what `org-babel-comint--echo-filter' and
`org-babel-comint-with-output' expect - the usual behavior with REPLs
echoing the input is displaying the whole (multiline) input together.

> ...
> Therefore, the adding "%-<org-eval>" MATLAB comments to the code being
> evaluated, then stripping the echo'd lines containing the "%-<org-eval>"
is
> probably the best option.

What I am thinking as an alternative is modifying
`org-babel-comint-with-output' to filter FULL-BODY line by line.
For example, when REMOVE-ECHO is set to 'line.

`org-babel-comint--echo-filter' can then be passed an extra argument
that will make it filter body lines rather than the whole body only.

WDYT?
[Quoted text hidden]
------------------------------
*John C *<john.ciolfi.32@gmail.com> Fri, Dec 27, 2024 at 8:24 AM

To: Ihor Radchenko <yantar92@posteo.net>
I'm not clear on how I'd update org-babel-comint-with-output to filter
line-by-line. Could you give me a start at it? Also, I'm not sure if
line-by-line filtering would work. I suspect MATLAB echos when it sees a
new line and a full statement. For example,

    m1 = [4*pi, 3*pi; ...
          2*pi, pi];

is a multi-line single statement. The "..." are MATLAB's line continuation
syntax.

Another thought is we could update org-babel-comint-with-output to have an
option like:

     (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY EOL-FILTER-INDICATOR)

where optional EOL-FILTER-INDICATOR is text, typically a comment, that is
appended to each line in FULL-BODY before it is evaluated. After
evaluation, each line in the result containing EOL-FILTER-INDICATOR is
removed from the result.

This way, if another language encounters this issue, they can leverage this
service.

Thanks
John
[Quoted text hidden]
------------------------------
*Ihor Radchenko *<yantar92@posteo.net> Fri, Dec 27, 2024 at 10:26 AM

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

> I'm not clear on how I'd update org-babel-comint-with-output to filter
> line-by-line. Could you give me a start at it?

For example, you can add an optional argument to
`org-babel-comint--echo-filter'. When it is passed, BODY argument is
split via `split-string' and then individual lines are removed in order
from STRING.

`org-babel-comint-with-output' is the caller of
`org-babel-comint--echo-filter'.

> ... Also, I'm not sure if
> line-by-line filtering would work. I suspect MATLAB echos when it sees a
> new line and a full statement. For example,
>
>     m1 = [4*pi, 3*pi; ...
>           2*pi, pi];
>
> is a multi-line single statement. The "..." are MATLAB's line continuation
> syntax.

AFAIU, removing line-by-line should still work in such case.

> Another thought is we could update org-babel-comint-with-output to have an
> option like:
>
>      (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY EOL-FILTER-INDICATOR)
>
> where optional EOL-FILTER-INDICATOR is text, typically a comment, that is
> appended to each line in FULL-BODY before it is evaluated. After
> evaluation, each line in the result containing EOL-FILTER-INDICATOR is
> removed from the result.

It may also work. Maybe even better.
Rather than just EOL-FILTER-INDICATOR, we may allow a regular expression
to be provided. Then, everything matching that regular expression will
be removed.

P.S. Is there any reason you are writing off-list? We generally try to
keep the discussion public to keep record of the decision-making.



On Mon, Dec 23, 2024 at 2:23 AM Ihor Radchenko <yantar92@posteo.net> wrote:

> Hi John,
>
> It has been a month since the last message in this thread.
> Do you need any help working on the patch?
>
> --
> Ihor Radchenko // yantar92,
> Org mode maintainer,
> 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>
>

[-- Attachment #1.2: Type: text/html, Size: 15793 bytes --]

[-- Attachment #2: org-matlab.patch --]
[-- Type: text/x-patch, Size: 18403 bytes --]

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 5d421172f..4eedba0ae 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -162,6 +162,23 @@ bibliography format requires them to be written in title-case.
 
 # This also includes changes in function behavior from Elisp perspective.
 
+*** ob-matlab: fixed MATLAB support
+
+Fixed MATLAB babel code blocks processing. MATLAB code blocks, ~#+begin_src matlab~, with ~:results
+verbatim~, ~:results output~, ~:results output latex~, or ~:results file graphics~ now work.  Fixes
+include (1) waiting for matlab-shell to start before evaluating MATLAB code, (2) correctly showing
+the results using writematrix, (3) removing the code block lines from the result, (4) correctly
+handling graphics results by invoking print correctly. To use MATLAB with org, you need
+https://github.com/MathWorks/Emacs-MATLAB-Mode.
+
+*** ob-matlab: MATLAB behavior change
+
+MATLAB code blocks now reuse the ~MATLAB*~ buffer created by ~M-x matlab-shell~, whereas the
+prior version started a new shell for each evaluation.  The benefit of this is that
+evaluations are very fast after the first evaluation and that state is maintained between
+evaluations, which you can clear using the MATLAB ~clear~ command.  Another benefit of this
+behavior is that it is consistent with how MATLAB works.
+
 *** ob-sqlite: Added ability to open a database in readonly mode
 
 Added option :readonly to ob-sqlite.
diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index b88ac445a..830e2245b 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -101,15 +101,28 @@ PROMPT-REGEXP defaults to `comint-prompt-regexp'."
        (setq string (substring string (match-end 0))))
   string)
 
+(defun org-babel-comint--strip-regexps (result strip-regexps)
+  "STRIP-REGEXPS from RESULT list of strings."
+  (dolist (strip-regexp strip-regexps)
+    (let ((new-result '()))
+      (dolist (line result)
+        (setq line (replace-regexp-in-string strip-regexp "" line))
+        (when (not (string= line ""))
+          (setq new-result (append new-result `(,line)))))
+      (setq result new-result)))
+  result)
+
 (defmacro org-babel-comint-with-output (meta &rest body)
   "Evaluate BODY in BUFFER and return process output.
 Will wait until EOE-INDICATOR appears in the output, then return
 all process output.  If REMOVE-ECHO and FULL-BODY are present and
-non-nil, then strip echo'd body from the returned output.  META
-should be a list containing the following where the last two
-elements are optional.
+non-nil, then strip echo'd body from the returned output.  If
+optional STRIP-REGEXPS, a list of regular expressions, is
+present, then all matches will be removed from the returned
+output.  META should be a list containing the following where the
+last three elements are optional.
 
- (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY)
+ (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY STRIP-REGEXPS)
 
 This macro ensures that the filter is removed in case of an error
 or user `keyboard-quit' during execution of body."
@@ -117,7 +130,8 @@ or user `keyboard-quit' during execution of body."
   (let ((buffer (nth 0 meta))
 	(eoe-indicator (nth 1 meta))
 	(remove-echo (nth 2 meta))
-	(full-body (nth 3 meta)))
+	(full-body (nth 3 meta))
+        (strip-regexps (nth 4 meta)))
     `(org-babel-comint-in-buffer ,buffer
        (let* ((string-buffer "")
 	      (comint-output-filter-functions
@@ -165,8 +179,12 @@ or user `keyboard-quit' during execution of body."
          (and ,remove-echo ,full-body
               (setq string-buffer (org-babel-comint--echo-filter string-buffer ,full-body)))
 
-         ;; Filter out prompts.
-         (org-babel-comint--prompt-filter string-buffer)))))
+         ;; Filter out prompts from returned output result.
+         (let ((result (org-babel-comint--prompt-filter string-buffer)))
+           ;; Remove all matches of STRIP-REGEXPS in returned output result.
+           (when ,strip-regexps
+             (setq result (org-babel-comint--strip-regexps result ,strip-regexps)))
+           result)))))
 
 (defun org-babel-comint-input-command (buffer cmd)
   "Pass CMD to BUFFER.
diff --git a/lisp/ob-matlab.el b/lisp/ob-matlab.el
index de8deadbe..083dcdec3 100644
--- a/lisp/ob-matlab.el
+++ b/lisp/ob-matlab.el
@@ -28,11 +28,10 @@
 
 ;;; Requirements:
 
-;; 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
+;; 2) https://github.com/mathworks/Emacs-MATLAB-Mode
+;;    For matlab-shell to run MATLAB within Emacs and matlab-mode
+;;    major mode for source code editing buffer
 
 ;;; Code:
 
diff --git a/lisp/ob-octave.el b/lisp/ob-octave.el
index 005990f20..024984835 100644
--- a/lisp/ob-octave.el
+++ b/lisp/ob-octave.el
@@ -1,4 +1,4 @@
-;;; ob-octave.el --- Babel Functions for Octave and Matlab -*- lexical-binding: t; -*-
+;;; ob-octave.el --- Babel Functions for Octave and MATLAB -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 2010-2024 Free Software Foundation, Inc.
 
@@ -30,6 +30,8 @@
 
 ;;; Code:
 
+(require 'cl-seq)
+
 (require 'org-macs)
 (org-assert-version)
 
@@ -39,7 +41,52 @@
 (declare-function matlab-shell "ext:matlab-mode")
 (declare-function matlab-shell-run-region "ext:matlab-mode")
 
-(defvar org-babel-default-header-args:matlab '())
+;; With `org-babel-default-header-args:matlab' set to
+;;  '((:session . "*MATLAB*")))
+;; each matlab code block evaluation will reuse the "*MATLAB*" buffer
+;; created by `matlab-shell' for code evaluation.  The benefit of this
+;; is that evaluation is very fast for evaluations after the first
+;; evaluation. The first evaluation may be slower because it can take
+;; a long time to start MATLAB.  Reusing the session buffer means
+;; state is maintained between evaluations.  To avoid reuse of state,
+;; you can clear the MATLAB workspace.  This setting aligns with
+;; typical MATLAB use, where MATLAB is started and used for a long
+;; period of period of time.  Another benefit of this setting is that
+;; you can see the history of the evaluations in the "*MATLAB*"
+;; command window buffer.  For example:
+;;
+;;  #+begin_src matlab :results output
+;;    a = 123
+;;  #+end_src
+;;
+;;  #+RESULTS:
+;;  : a =
+;;  :
+;;  :    123
+;;
+;;  #+begin_src matlab :results output
+;;    b = a * 2
+;;  #+end_src
+;;
+;;  #+RESULTS:
+;;  : b =
+;;  :
+;;  :    246
+;;
+;;  #+begin_src matlab :results output
+;;    clear
+;;    c = b * 2
+;;  #+end_src
+;;
+;;  #+RESULTS:
+;;  : Unrecognized function or variable 'b'.
+;;
+;; If you want a new session each time you evaluate a MATLAB code block,
+;;   (setq 'org-babel-default-header-args:matlab '())
+;; However, this will make evaluations slower and is not consistent with how
+;; MATLAB works.  MATLAB is designed for many evaluations.
+(defvar org-babel-default-header-args:matlab '((:session . "*MATLAB*")))
+
 (defvar org-babel-default-header-args:octave '())
 
 (defvar org-babel-matlab-shell-command "matlab -nosplash"
@@ -47,18 +94,42 @@
 (defvar org-babel-octave-shell-command "octave -q"
   "Shell command to run octave as an external process.")
 
-(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')
+(make-obsolete-variable 'org-babel-matlab-with-emacs-link
+                        "MATLAB removed EmacsLink in R2009a." "2009")
+
+(make-obsolete-variable 'org-babel-matlab-emacs-link-wrapper-method
+                        "MATLAB removed EmacsLink in R2009a." "2009")
+
+(defvar org-babel-matlab-print "print(\"-dpng\", %S);\nans=%S;"
+  ;; MATLAB command-function duality requires that the file name be specified
+  ;; without quotes. Using: print -dpng "file.png", would produce a file with
+  ;; the quotes in the file name on disk. Therefore, use the functional form
+  ;; to handle files with spaces, print("-dpng", "file.png").
+  ;; Example:
+  ;;    #+begin_src matlab :file "sine wave.png" :results file graphics
+  ;;      t = [0 : 0.1 : 2*pi];
+  ;;      y = sin(t);
+  ;;      plot(t, y);
+  ;;      set(gcf, 'PaperUnits', 'inches', 'PaperPosition', [0 0 4 3]) % Set the size to 4" x 3"
+  ;;    #+end_src
+  ;;
+  ;;    #+RESULTS:
+  ;;    [[file:sine wave.png]]
+  "MATLAB format specifier to print current figure to a file.")
+
+(defvar org-babel-octave-print "print -dpng %S\nans=%S"
+  "Octave format specifier to print current figure to a file.")
+
+(defvar org-babel-matlab-wrapper-method
+  (concat "\
+cd('%s');
+%s
+if ~exist('ans', 'var') ans = ''; end; \
+writematrix(ans, '%s', 'Delimiter', 'tab');
 ")
+  "Format specifier used when evaluating MATLAB code blocks.
+Arguments are the `default-directory', the MATLAB code, and a result file.txt.")
+
 (defvar org-babel-octave-wrapper-method
   "%s
 if ischar(ans), fid = fopen('%s', 'w'); fdisp(fid, ans); fclose(fid);
@@ -92,7 +163,10 @@ When MATLABP is non-nil, execute Matlab.  Otherwise, execute Octave."
 				 (list
 				  "set (0, \"defaultfigurevisible\", \"off\");"
 				  full-body
-				  (format "print -dpng %S\nans=%S" gfx-file gfx-file))
+				  (format (if matlabp
+                                              org-babel-matlab-print
+                                            org-babel-octave-print)
+                                          gfx-file gfx-file))
 				 "\n")
 		    full-body)
 		  result-type matlabp)))
@@ -153,6 +227,23 @@ If there is not a current inferior-process-buffer in SESSION then
 create.  Return the initialized session.  PARAMS are src block parameters."
   (org-babel-octave-initiate-session session params 'matlab))
 
+(defun org-babel-matlab-shell ()
+  "Start and/or wait for MATLAB shell."
+  (require 'matlab-shell) ;; make `matlab-shell-busy-checker' available
+  (cond
+   ((fboundp 'matlab-shell-busy-checker)
+    ;; Start the shell if needed.  `matlab-shell' will reuse existing if already running.
+    (matlab-shell)
+    ;; If we just started the matlab-shell, wait for the prompt.  If we do not
+    ;; wait, then the startup messages will show up in the evaluation results.
+    (matlab-shell-busy-checker 'wait-for-prompt))
+   (t
+    (message (concat "You version of matlab-mode is old.\n"
+                     "Please update, see https://github.com/mathworks/Emacs-MATLAB-Mode\n"
+                     "Updating will eliminate unexpected output in your results\n"))
+    (sit-for 3)
+    (matlab-shell))))
+
 (defun org-babel-octave-initiate-session (&optional session _params matlabp)
   "Create an octave inferior process buffer.
 If there is not a current inferior-process-buffer in SESSION then
@@ -165,9 +256,15 @@ Octave session, unless MATLABP is non-nil."
   (unless (string= session "none")
     (let ((session (or session
 		       (if matlabp "*Inferior Matlab*" "*Inferior Octave*"))))
-      (if (org-babel-comint-buffer-livep session) session
+      (if (org-babel-comint-buffer-livep session)
+          (progn
+            (when (and matlabp (fboundp 'matlab-shell-busy-checker))
+              ;; Can't evaluate if the matlab-shell is currently running code
+              (matlab-shell-busy-checker 'error-if-busy))
+            session)
 	(save-window-excursion
-	  (if matlabp (unless org-babel-matlab-with-emacs-link (matlab-shell))
+	  (if matlabp
+              (org-babel-matlab-shell)
 	    (run-octave))
 	  (rename-buffer (if (bufferp session) (buffer-name session)
 			   (if (stringp session) session (buffer-name))))
@@ -183,66 +280,104 @@ value of the last statement in BODY, as elisp."
       (org-babel-octave-evaluate-session session body result-type matlabp)
     (org-babel-octave-evaluate-external-process body result-type matlabp)))
 
+(defun org-babel-octave-wrapper-tmp-file (matlabp)
+  "Return a local tmp file with name adjusted for MATLABP."
+  (if matlabp
+      ;; writematrix requires a file ending with '.txt'
+      (org-babel-temp-file "matlab-" ".txt")
+    (org-babel-temp-file "octave-")))
+
+(defun org-babel-octave-get-code-to-eval (body tmp-file matlabp)
+  "Format BODY of the code block for evaluation saving results to TMP-FILE.
+If MATLABP, format for MATLAB, else format for Octave."
+    (if matlabp
+        (format org-babel-matlab-wrapper-method default-directory body tmp-file)
+      (format org-babel-octave-wrapper-method body tmp-file tmp-file)))
+
 (defun org-babel-octave-evaluate-external-process (body result-type matlabp)
-  "Evaluate BODY in an external Octave or Matalab process.
+  "Evaluate BODY in an external Octave or MATLAB process.
 Process the result as RESULT-TYPE.  Use Octave, unless MATLABP is non-nil."
   (let ((cmd (if matlabp
 		 org-babel-matlab-shell-command
 	       org-babel-octave-shell-command)))
     (pcase result-type
       (`output (org-babel-eval cmd body))
-      (`value (let ((tmp-file (org-babel-temp-file "octave-")))
+      (`value (let ((tmp-file (org-babel-process-file-name
+                               (org-babel-octave-wrapper-tmp-file matlabp)
+                               'noquote)))
 	        (org-babel-eval
 		 cmd
-		 (format org-babel-octave-wrapper-method body
-			 (org-babel-process-file-name tmp-file 'noquote)
-			 (org-babel-process-file-name tmp-file 'noquote)))
+                 (org-babel-octave-get-code-to-eval body tmp-file matlabp))
 	        (org-babel-octave-import-elisp-from-file tmp-file))))))
 
+(defun org-babel-body-for-output (body matlabp)
+  "If MATLABP, fixup BODY for MATLAB output result-type."
+  (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
+    ;; remove these unwanted lines, we append a comment "%-<org-eval>" to each
+    ;; line in the body MATLAB code.  After we collect the results from
+    ;; evaluation, we leverage the "%-<org-eval>" to remove the unwanted lines.
+    ;; Example of desired behavior:
+    ;;     #+begin_src matlab :results output
+    ;;       disp('The results are:')
+    ;;       a = [1, 2; 3, 4]
+    ;;       b = a * 2
+    ;;     #+end_src
+    ;;
+    ;;     #+RESULTS:
+    ;;     #+begin_example
+    ;;     The results are:
+    ;;
+    ;;     a =
+    ;;
+    ;;          1     2
+    ;;          3     4
+    ;;
+    ;;     b =
+    ;;
+    ;;          2     4
+    ;;          6     8
+    ;;     #+end_example
+    (setq body (replace-regexp-in-string "\n" " %-<org-eval>-\n" body))
+    (when (not (string-match "\n\\'" body))
+      (setq body (concat body " %-<org-eval>-"))))
+  body)
+
 (defun org-babel-octave-evaluate-session
     (session body result-type &optional matlabp)
   "Evaluate BODY in SESSION."
-  (let* ((tmp-file (org-babel-temp-file (if matlabp "matlab-" "octave-")))
-	 (wait-file (org-babel-temp-file "matlab-emacs-link-wait-signal-"))
+  (let* ((tmp-file (org-babel-octave-wrapper-tmp-file matlabp))
 	 (full-body
 	  (pcase result-type
 	    (`output
 	     (mapconcat
 	      #'org-babel-chomp
-	      (list body org-babel-octave-eoe-indicator) "\n"))
+	      (list (org-babel-body-for-output body matlabp)
+                    org-babel-octave-eoe-indicator)
+              "\n"))
 	    (`value
-	     (if (and matlabp org-babel-matlab-with-emacs-link)
-		 (concat
-		  (format org-babel-matlab-emacs-link-wrapper-method
-			  body
-			  (org-babel-process-file-name tmp-file 'noquote)
-			  (org-babel-process-file-name tmp-file 'noquote) wait-file) "\n")
-	       (mapconcat
-		#'org-babel-chomp
-		(list (format org-babel-octave-wrapper-method
-			      body
-			      (org-babel-process-file-name tmp-file 'noquote)
-			      (org-babel-process-file-name tmp-file 'noquote))
-		      org-babel-octave-eoe-indicator) "\n")))))
-	 (raw (if (and matlabp org-babel-matlab-with-emacs-link)
-		  (save-window-excursion
-		    (with-temp-buffer
-		      (insert full-body)
-		      (write-region "" 'ignored wait-file nil nil nil 'excl)
-		      (matlab-shell-run-region (point-min) (point-max))
-		      (message "Waiting for Matlab Emacs Link")
-		      (while (file-exists-p wait-file) (sit-for 0.01))
-		      "")) ;; matlab-shell-run-region doesn't seem to
-		;; make *matlab* buffer contents easily
-		;; available, so :results output currently
-		;; won't work
-		(org-babel-comint-with-output
-		    (session
-		     (if matlabp
-			 org-babel-octave-eoe-indicator
-		       org-babel-octave-eoe-output)
-		     t full-body)
-		  (insert full-body) (comint-send-input nil t))))
+	     (mapconcat
+	      #'org-babel-chomp
+	      (list (org-babel-octave-get-code-to-eval body tmp-file matlabp)
+		    org-babel-octave-eoe-indicator)
+              "\n"))))
+	 (raw-results
+	  (org-babel-comint-with-output
+	      (session
+	       (if matlabp
+		   org-babel-octave-eoe-indicator
+		 org-babel-octave-eoe-output)
+	       t full-body ;; Remove echo'd full-body from result
+               (when matlabp
+                 '(;; MATLAB echo's all input lines, so use the %-<org-eval> comments to strip
+                   ;; them from the output
+                   "^[^\n]*%-<org-eval>-\n"
+                   ;; Remove starting blank line caused by stripping %-<org-eval>
+                   "\\`[[:space:]\r\n]+"
+                   ;; Strip <ERRORTXT> and </ERRORTXT> matlab-shell error indicators
+                   "</?ERRORTXT>\n")))
+	    (insert full-body) (comint-send-input nil t)))
 	 results)
     (pcase result-type
       (`value
@@ -250,11 +385,11 @@ Process the result as RESULT-TYPE.  Use Octave, unless MATLABP is non-nil."
       (`output
        (setq results
 	     (if matlabp
-		 (cdr (reverse (delete "" (mapcar #'org-strip-quotes
-					          (mapcar #'org-trim raw)))))
+                 (cdr (reverse (delete "" (mapcar #'org-strip-quotes
+					          (mapcar #'org-trim raw-results)))))
 	       (cdr (member org-babel-octave-eoe-output
 			    (reverse (mapcar #'org-strip-quotes
-					     (mapcar #'org-trim raw)))))))
+					     (mapcar #'org-trim raw-results)))))))
        (mapconcat #'identity (reverse results) "\n")))))
 
 (defun org-babel-octave-import-elisp-from-file (file-name)

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: ob-octave: improve MATLAB support
  2024-12-29  3:04         ` John C
@ 2024-12-29  7:42           ` Ihor Radchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Ihor Radchenko @ 2024-12-29  7:42 UTC (permalink / raw)
  To: John C; +Cc: emacs-orgmode

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

> See attached org-matlab.patch which addresses all feedback. Here's the
> commit info.

Thanks!
See my comments inline.

> +*** ob-matlab: fixed MATLAB support
> +
> +Fixed MATLAB babel code blocks processing. MATLAB code blocks, ~#+begin_src matlab~, with ~:results
> +verbatim~, ~:results output~, ~:results output latex~, or ~:results file graphics~ now work.  Fixes
> +include (1) waiting for matlab-shell to start before evaluating MATLAB code, (2) correctly showing
> +the results using writematrix, (3) removing the code block lines from the result, (4) correctly
> +handling graphics results by invoking print correctly. To use MATLAB with org, you need
> +https://github.com/MathWorks/Emacs-MATLAB-Mode.

There is no need to provide so many details.
Just leave the most important things:

1. MATLAB is no longer broken
2. Emacs-MATLAB-Mode is required now

> +*** ob-matlab: MATLAB behavior change
> +
> +MATLAB code blocks now reuse the ~MATLAB*~ buffer created by ~M-x matlab-shell~, whereas the
> +prior version started a new shell for each evaluation.  The benefit of this is that
> +evaluations are very fast after the first evaluation and that state is maintained between
> +evaluations, which you can clear using the MATLAB ~clear~ command.  Another benefit of this
> +behavior is that it is consistent with how MATLAB works.

No need to explain in so much details, I think.
Just say that MATLAB uses session by default and them mention that users
may customize `org-babel-default-header-args:matlab' to disable session.

> +(defun org-babel-comint--strip-regexps (result strip-regexps)
> +  "STRIP-REGEXPS from RESULT list of strings."
> +  (dolist (strip-regexp strip-regexps)
> +    (let ((new-result '()))
> +      (dolist (line result)
> +        (setq line (replace-regexp-in-string strip-regexp "" line))
> +        (when (not (string= line ""))
> +          (setq new-result (append new-result `(,line)))))

It is more efficient to use `push' + `nreverse' instead of `append'.

> -(defvar org-babel-default-header-args:matlab '())
> +;; With `org-babel-default-header-args:matlab' set to
> +;;  '((:session . "*MATLAB*")))
> +;; ...
> +;; If you want a new session each time you evaluate a MATLAB code block,
> +;;   (setq 'org-babel-default-header-args:matlab '())
> +;; However, this will make evaluations slower and is not consistent with how
> +;; MATLAB works.  MATLAB is designed for many evaluations.
> +(defvar org-babel-default-header-args:matlab '((:session . "*MATLAB*")))

You don't need that long comment in the source code.
If you think that explaining the details about session is necessary (it
may or may not be, but we should assume that Org users are familiar with
the notion of sessions in code blocks), please do it in the
documentation, not in the code.

More generally, your motivation is not specific to matlab. Yet, we
default to no session in most babel backends. So, it is not a question
of session being faster or slower, but a question of consistency.

That said, some babel backends do default to session, so I do not oppose
this change too much.

> +(make-obsolete-variable 'org-babel-matlab-with-emacs-link
> +                        "MATLAB removed EmacsLink in R2009a." "2009")
> +
> +(make-obsolete-variable 'org-babel-matlab-emacs-link-wrapper-method
> +                        "MATLAB removed EmacsLink in R2009a." "2009")

Please use Org version in WHEN argument of `make-obsolete-variable'.
The WHEN should be "9.8".

> +(defun org-babel-matlab-shell ()
> +  "Start and/or wait for MATLAB shell."
> +  (require 'matlab-shell) ;; make `matlab-shell-busy-checker' available
> +  (cond
> +   ((fboundp 'matlab-shell-busy-checker)
> +    ;; Start the shell if needed.  `matlab-shell' will reuse existing if already running.
> +    (matlab-shell)
> +    ;; If we just started the matlab-shell, wait for the prompt.  If we do not
> +    ;; wait, then the startup messages will show up in the evaluation results.
> +    (matlab-shell-busy-checker 'wait-for-prompt))
> +   (t
> +    (message (concat "You version of matlab-mode is old.\n"
> +                     "Please update, see https://github.com/mathworks/Emacs-MATLAB-Mode\n"
> +                     "Updating will eliminate unexpected output in your results\n"))
> +    (sit-for 3)

Instead of `message' + `sit-fit', you can simply use `display-warning'.
It will give users more control.

> +(defun org-babel-body-for-output (body matlabp)
> +  "If MATLABP, fixup BODY for MATLAB output result-type."
> +  (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
> +    ;; remove these unwanted lines, we append a comment "%-<org-eval>" to each
> +    ;; line in the body MATLAB code.  After we collect the results from
> +    ;; evaluation, we leverage the "%-<org-eval>" to remove the unwanted lines.
> +    ;; Example of desired behavior:
> ...

I think that an important point here is that MATLAB does not echo the
whole BODY all at once and instead mixes it with the output. Which is
why we need to do something non-standard to filter out the body in
matlab specifically.

> +    (setq body (replace-regexp-in-string "\n" " %-<org-eval>-\n" body))
> +    (when (not (string-match "\n\\'" body))
> +      (setq body (concat body " %-<org-eval>-"))))
> +  body)

Please put this %-<org-eval> into an internal constant and then reuse
it when building the regexp to filter.

> +               (when matlabp
> +                 '(;; MATLAB echo's all input lines, so use the %-<org-eval> comments to strip
> +                   ;; them from the output
> +                   "^[^\n]*%-<org-eval>-\n"
> +                   ;; Remove starting blank line caused by stripping %-<org-eval>
> +                   "\\`[[:space:]\r\n]+"
> +                   ;; Strip <ERRORTXT> and </ERRORTXT> matlab-shell error indicators
> +                   "</?ERRORTXT>\n")))

Same here. Please put these regexps into a constant.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-12-29  7:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-08 19:30 ob-octave: improve MATLAB support John C
2024-11-09  9:32 ` Ihor Radchenko
2024-11-13 19:10   ` John C
2024-11-15 13:29     ` John C
2024-11-23 15:57     ` Ihor Radchenko
     [not found]       ` <87o712lv3t.fsf@localhost>
2024-12-29  3:04         ` John C
2024-12-29  7:42           ` Ihor Radchenko

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