bug#45795: 27.1; prolog-mode prolog-system switch bug

bug#45795: 27.1; prolog-mode prolog-system switch bug

[k3tu0isui]

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

Switching between prolog systems is not supported properly.

Say I stated with (setq prolog-system 'swi) in my configuration, and I set (setq-local prolog-system 'gnu) in my working buffer. Now when I do `run-prolog` it is expected to run gprolog instead of swipl. But this is not happening, it still runs swipl.

The bug seems to be in `prolog-ensure-process` function. If the process is not running, a new process is created using make-comint-in-buffer whose program name is supplied using (prolog-program-name). But we have already shifted to the *prolog* buffer here, so whatever the value of prolog-system in our source buffer, the program started is still that of the global variable.

In my patch I just bound (prolog-program-name) and (prolog-program-switches) before switching to *prolog* buffer. This seems to work.

[-- Attachment #2: 0001-prolog-mode-prolog-system-switch-process-bug.patch --]
[-- Type: text/plain, Size: 5459 bytes --]

From d9770ecac117fedd3d6351144a6f1464f701fd73 Mon Sep 17 00:00:00 2001
From: keutoi <k3tu0isui@gmail.com>
Date: Mon, 11 Jan 2021 22:34:20 +0530
Subject: [PATCH] prolog-mode prolog-system switch process bug

---
 lisp/progmodes/prolog.el | 102 ++++++++++++++++++++-------------------
 1 file changed, 52 insertions(+), 50 deletions(-)

diff --git a/lisp/progmodes/prolog.el b/lisp/progmodes/prolog.el
index 9f5f9ed6d3..f866f1e2f6 100644
--- a/lisp/progmodes/prolog.el
+++ b/lisp/progmodes/prolog.el
@@ -1350,56 +1350,58 @@ prolog-ensure-process
   "If Prolog process is not running, run it.
 If the optional argument WAIT is non-nil, wait for Prolog prompt specified by
 the variable `prolog-prompt-regexp'."
-  (if (null (prolog-program-name))
-      (error "This Prolog system has defined no interpreter."))
-  (if (comint-check-proc "*prolog*")
-      ()
-    (with-current-buffer (get-buffer-create "*prolog*")
-      (prolog-inferior-mode)
-
-      ;; The "INFERIOR=yes" hack is for SWI-Prolog 7.2.3 and earlier,
-      ;; which assumes it is running under Emacs if either INFERIOR=yes or
-      ;; if EMACS is set to a nonempty value.  The EMACS setting is
-      ;; obsolescent, so set INFERIOR.  Newer versions of SWI-Prolog should
-      ;; know about INSIDE_EMACS (which replaced EMACS) and should not need
-      ;; this hack.
-      (let ((process-environment
-	     (if (getenv "INFERIOR")
-		 process-environment
-	       (cons "INFERIOR=yes" process-environment))))
-	(apply 'make-comint-in-buffer "prolog" (current-buffer)
-	       (prolog-program-name) nil (prolog-program-switches)))
-
-      (unless prolog-system
-        ;; Setup auto-detection.
-        (setq-local
-         prolog-system
-         ;; Force re-detection.
-         (let* ((proc (get-buffer-process (current-buffer)))
-                (pmark (and proc (marker-position (process-mark proc)))))
-           (cond
-            ((null pmark) (1- (point-min)))
-            ;; The use of insert-before-markers in comint.el together with
-            ;; the potential use of comint-truncate-buffer in the output
-            ;; filter, means that it's difficult to reliably keep track of
-            ;; the buffer position where the process's output started.
-            ;; If possible we use a marker at "start - 1", so that
-            ;; insert-before-marker at `start' won't shift it.  And if not,
-            ;; we fall back on using a plain integer.
-            ((> pmark (point-min)) (copy-marker (1- pmark)))
-            (t (1- pmark)))))
-        (add-hook 'comint-output-filter-functions
-                  'prolog-inferior-guess-flavor nil t))
-      (if wait
-          (progn
-            (goto-char (point-max))
-            (while
-                (save-excursion
-                  (not
-                   (re-search-backward
-                    (concat "\\(" (prolog-prompt-regexp) "\\)" "\\=")
-                    nil t)))
-              (sit-for 0.1)))))))
+  (let ((pname (prolog-program-name))
+        (pswitches (prolog-program-switches)))
+    (if (null pname)
+        (error "This Prolog system has defined no interpreter."))
+    (if (comint-check-proc "*prolog*")
+        ()
+      (with-current-buffer (get-buffer-create "*prolog*")
+        (prolog-inferior-mode)
+
+        ;; The "INFERIOR=yes" hack is for SWI-Prolog 7.2.3 and earlier,
+        ;; which assumes it is running under Emacs if either INFERIOR=yes or
+        ;; if EMACS is set to a nonempty value.  The EMACS setting is
+        ;; obsolescent, so set INFERIOR.  Newer versions of SWI-Prolog should
+        ;; know about INSIDE_EMACS (which replaced EMACS) and should not need
+        ;; this hack.
+        (let ((process-environment
+	       (if (getenv "INFERIOR")
+		   process-environment
+	         (cons "INFERIOR=yes" process-environment))))
+	  (apply 'make-comint-in-buffer "prolog" (current-buffer)
+	         pname nil pswitches))
+
+        (unless prolog-system
+          ;; Setup auto-detection.
+          (setq-local
+           prolog-system
+           ;; Force re-detection.
+           (let* ((proc (get-buffer-process (current-buffer)))
+                  (pmark (and proc (marker-position (process-mark proc)))))
+             (cond
+              ((null pmark) (1- (point-min)))
+              ;; The use of insert-before-markers in comint.el together with
+              ;; the potential use of comint-truncate-buffer in the output
+              ;; filter, means that it's difficult to reliably keep track of
+              ;; the buffer position where the process's output started.
+              ;; If possible we use a marker at "start - 1", so that
+              ;; insert-before-marker at `start' won't shift it.  And if not,
+              ;; we fall back on using a plain integer.
+              ((> pmark (point-min)) (copy-marker (1- pmark)))
+              (t (1- pmark)))))
+          (add-hook 'comint-output-filter-functions
+                    'prolog-inferior-guess-flavor nil t))
+        (if wait
+            (progn
+              (goto-char (point-max))
+              (while
+                  (save-excursion
+                    (not
+                     (re-search-backward
+                      (concat "\\(" (prolog-prompt-regexp) "\\)" "\\=")
+                      nil t)))
+                (sit-for 0.1))))))))
 
 (defun prolog-inferior-buffer (&optional dont-run)
   (or (get-buffer "*prolog*")
-- 
2.29.2

bug#45795: prolog-mode prolog-system switch bug - also with run-prolog

[k3tu0isui]

I have also noticed that this affects the `run-prolog` function in a way not affected by my previous patch.
In `run-prolog` function `prolog-ensure-process` is called after switching to the *prolog* buffer using `prolog-goto-prolog-process-buffer`. So my previous patch which deals with `prolog-ensure-process` will not affect this. I do not know how to rectify this due to the presence of SICStus-Prolog specific code that seems to be working in *prolog* buffer. If I moved `prolog-ensure-process` call before switching to *prolog* buffer, something SICStus might break. I have never worked with SICStus so I am not sure how to deal with this.

bug#45795: [PATCH] prolog-mode run-prolog and prolog-consult-file fixes when prolog-system changes

[k3tu0isui]

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

Can someone working with SICStus check if this patch breaks debugger(or anything) when `run-prolog` is used?

I have repeatedly checked switching between GNU-Prolog and SWI-Prolog systems after setting `prolog-system` to buffer local values 'gnu' and 'swi' resp.
Both `prolog-consult-file` and `run-prolog` are now starting the correct interpreter.


[-- Attachment #2: whitespace ignored diff --]
[-- Type: text/plain, Size: 1865 bytes --]

diff --git a/lisp/progmodes/prolog.el b/lisp/progmodes/prolog.el
index 9f5f9ed6d3..00d865af1a 100644
--- a/lisp/progmodes/prolog.el
+++ b/lisp/progmodes/prolog.el
@@ -1316,6 +1316,7 @@ run-prolog
       (progn
         (process-send-string "prolog" "halt.\n")
         (while (get-process "prolog") (sit-for 0.1))))
+  (prolog-ensure-process)
   (let ((buff (buffer-name)))
     (if (not (string= buff "*prolog*"))
         (prolog-goto-prolog-process-buffer))
@@ -1325,7 +1326,6 @@ run-prolog
              prolog-use-sicstus-sd)
         (prolog-enable-sicstus-sd))
     (prolog-mode-variables)
-    (prolog-ensure-process)
     ))
 
 (defun prolog-inferior-guess-flavor (&optional ignored)
@@ -1350,7 +1350,9 @@ prolog-ensure-process
   "If Prolog process is not running, run it.
 If the optional argument WAIT is non-nil, wait for Prolog prompt specified by
 the variable `prolog-prompt-regexp'."
-  (if (null (prolog-program-name))
+  (let ((pname (prolog-program-name))
+        (pswitches (prolog-program-switches)))
+    (if (null pname)
         (error "This Prolog system has defined no interpreter."))
     (if (comint-check-proc "*prolog*")
         ()
@@ -1368,7 +1370,7 @@ prolog-ensure-process
 		   process-environment
 	         (cons "INFERIOR=yes" process-environment))))
 	  (apply 'make-comint-in-buffer "prolog" (current-buffer)
-	       (prolog-program-name) nil (prolog-program-switches)))
+	         pname nil pswitches))
 
         (unless prolog-system
           ;; Setup auto-detection.
@@ -1399,7 +1401,7 @@ prolog-ensure-process
                      (re-search-backward
                       (concat "\\(" (prolog-prompt-regexp) "\\)" "\\=")
                       nil t)))
-              (sit-for 0.1)))))))
+                (sit-for 0.1))))))))
 
 (defun prolog-inferior-buffer (&optional dont-run)
   (or (get-buffer "*prolog*")

[-- Attachment #3: prolog-switch-final.diff --]
[-- Type: text/plain, Size: 5642 bytes --]

diff --git a/lisp/progmodes/prolog.el b/lisp/progmodes/prolog.el
index 9f5f9ed6d3..00d865af1a 100644
--- a/lisp/progmodes/prolog.el
+++ b/lisp/progmodes/prolog.el
@@ -1316,6 +1316,7 @@ run-prolog
       (progn
         (process-send-string "prolog" "halt.\n")
         (while (get-process "prolog") (sit-for 0.1))))
+  (prolog-ensure-process)
   (let ((buff (buffer-name)))
     (if (not (string= buff "*prolog*"))
         (prolog-goto-prolog-process-buffer))
@@ -1325,7 +1326,6 @@ run-prolog
              prolog-use-sicstus-sd)
         (prolog-enable-sicstus-sd))
     (prolog-mode-variables)
-    (prolog-ensure-process)
     ))
 
 (defun prolog-inferior-guess-flavor (&optional ignored)
@@ -1350,56 +1350,58 @@ prolog-ensure-process
   "If Prolog process is not running, run it.
 If the optional argument WAIT is non-nil, wait for Prolog prompt specified by
 the variable `prolog-prompt-regexp'."
-  (if (null (prolog-program-name))
-      (error "This Prolog system has defined no interpreter."))
-  (if (comint-check-proc "*prolog*")
-      ()
-    (with-current-buffer (get-buffer-create "*prolog*")
-      (prolog-inferior-mode)
-
-      ;; The "INFERIOR=yes" hack is for SWI-Prolog 7.2.3 and earlier,
-      ;; which assumes it is running under Emacs if either INFERIOR=yes or
-      ;; if EMACS is set to a nonempty value.  The EMACS setting is
-      ;; obsolescent, so set INFERIOR.  Newer versions of SWI-Prolog should
-      ;; know about INSIDE_EMACS (which replaced EMACS) and should not need
-      ;; this hack.
-      (let ((process-environment
-	     (if (getenv "INFERIOR")
-		 process-environment
-	       (cons "INFERIOR=yes" process-environment))))
-	(apply 'make-comint-in-buffer "prolog" (current-buffer)
-	       (prolog-program-name) nil (prolog-program-switches)))
-
-      (unless prolog-system
-        ;; Setup auto-detection.
-        (setq-local
-         prolog-system
-         ;; Force re-detection.
-         (let* ((proc (get-buffer-process (current-buffer)))
-                (pmark (and proc (marker-position (process-mark proc)))))
-           (cond
-            ((null pmark) (1- (point-min)))
-            ;; The use of insert-before-markers in comint.el together with
-            ;; the potential use of comint-truncate-buffer in the output
-            ;; filter, means that it's difficult to reliably keep track of
-            ;; the buffer position where the process's output started.
-            ;; If possible we use a marker at "start - 1", so that
-            ;; insert-before-marker at `start' won't shift it.  And if not,
-            ;; we fall back on using a plain integer.
-            ((> pmark (point-min)) (copy-marker (1- pmark)))
-            (t (1- pmark)))))
-        (add-hook 'comint-output-filter-functions
-                  'prolog-inferior-guess-flavor nil t))
-      (if wait
-          (progn
-            (goto-char (point-max))
-            (while
-                (save-excursion
-                  (not
-                   (re-search-backward
-                    (concat "\\(" (prolog-prompt-regexp) "\\)" "\\=")
-                    nil t)))
-              (sit-for 0.1)))))))
+  (let ((pname (prolog-program-name))
+        (pswitches (prolog-program-switches)))
+    (if (null pname)
+        (error "This Prolog system has defined no interpreter."))
+    (if (comint-check-proc "*prolog*")
+        ()
+      (with-current-buffer (get-buffer-create "*prolog*")
+        (prolog-inferior-mode)
+
+        ;; The "INFERIOR=yes" hack is for SWI-Prolog 7.2.3 and earlier,
+        ;; which assumes it is running under Emacs if either INFERIOR=yes or
+        ;; if EMACS is set to a nonempty value.  The EMACS setting is
+        ;; obsolescent, so set INFERIOR.  Newer versions of SWI-Prolog should
+        ;; know about INSIDE_EMACS (which replaced EMACS) and should not need
+        ;; this hack.
+        (let ((process-environment
+	       (if (getenv "INFERIOR")
+		   process-environment
+	         (cons "INFERIOR=yes" process-environment))))
+	  (apply 'make-comint-in-buffer "prolog" (current-buffer)
+	         pname nil pswitches))
+
+        (unless prolog-system
+          ;; Setup auto-detection.
+          (setq-local
+           prolog-system
+           ;; Force re-detection.
+           (let* ((proc (get-buffer-process (current-buffer)))
+                  (pmark (and proc (marker-position (process-mark proc)))))
+             (cond
+              ((null pmark) (1- (point-min)))
+              ;; The use of insert-before-markers in comint.el together with
+              ;; the potential use of comint-truncate-buffer in the output
+              ;; filter, means that it's difficult to reliably keep track of
+              ;; the buffer position where the process's output started.
+              ;; If possible we use a marker at "start - 1", so that
+              ;; insert-before-marker at `start' won't shift it.  And if not,
+              ;; we fall back on using a plain integer.
+              ((> pmark (point-min)) (copy-marker (1- pmark)))
+              (t (1- pmark)))))
+          (add-hook 'comint-output-filter-functions
+                    'prolog-inferior-guess-flavor nil t))
+        (if wait
+            (progn
+              (goto-char (point-max))
+              (while
+                  (save-excursion
+                    (not
+                     (re-search-backward
+                      (concat "\\(" (prolog-prompt-regexp) "\\)" "\\=")
+                      nil t)))
+                (sit-for 0.1))))))))
 
 (defun prolog-inferior-buffer (&optional dont-run)
   (or (get-buffer "*prolog*")

bug#45795: 27.1; prolog-mode prolog-system switch bug

[Lars Ingebrigtsen]

k3tu0isui@gmail.com writes:

> I have repeatedly checked switching between GNU-Prolog and SWI-Prolog
> systems after setting `prolog-system` to buffer local values 'gnu' and
> 'swi' resp.
> Both `prolog-consult-file` and `run-prolog` are now starting the
> correct interpreter.

I don't use Prolog myself, but your patch seems to make sense to me, so
I've applied it to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no