bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

[Nicholas Vollmer]

string-edit-mode is a nice feature to have built in.
It would be nice if one could change the major mode of the editing 
buffer while preserving the edit/abort functionality and any 
buffer state necessary to make those function.
Any reason it couldn't be implemented as a  minor mode?

bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

[Eli Zaretskii]

> From: Nicholas Vollmer <nv@parenthetic.dev>
> Date: Thu, 06 Jun 2024 11:50:16 -0400
> 
> 
> string-edit-mode is a nice feature to have built in.

It already is?  Or what do you mean by "built in" here?

> It would be nice if one could change the major mode of the editing 
> buffer while preserving the edit/abort functionality and any 
> buffer state necessary to make those function.
> Any reason it couldn't be implemented as a  minor mode?

In what major modes will this make sense as a minor mode?

IOW, I don't think I understand what you mean by "preserving the
edit/abort functionality and buffer state", so please elaborate.

bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

[Nicholas Vollmer]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Nicholas Vollmer <nv@parenthetic.dev>
>> Date: Thu, 06 Jun 2024 11:50:16 -0400
>> 
>> 
>> string-edit-mode is a nice feature to have built in.
>
> It already is?  Or what do you mean by "built in" here?

Correct. It is. I was just commenting that I'm glad to see it part 
of Emacs, rather than re-implemented over and over in third-party 
packages.
That is all.
 
>> It would be nice if one could change the major mode of the 
>> editing 
>> buffer while preserving the edit/abort functionality and any 
>> buffer state necessary to make those function.
>> Any reason it couldn't be implemented as a  minor mode?
>
> In what major modes will this make sense as a minor mode?

Many. For example. If the string to edit is an elisp form, why 
shouldn't the editing buffer be emacs-lisp-mode?
If the string to edit is a message which can be formatted with 
markdown, why not markdown-mode?
(I ran into this yesterday when implementing a command for 
personal use).
I can imagine many scenarios where the major mode could be 
something other than text-mode.
It seems to me that it would make more sense for the 
"abort/finish" key bindings to be bound to  a minor mode map so 
the editing buffer can be much more flexible.
 
> IOW, I don't think I understand what you mean by "preserving the
> edit/abort functionality and buffer state", so please elaborate.

The buffer has some local state (e.g. the abort callback function, 
header-line) stored in a buffer-local variables which are not 
preserved when the major mode is changed.

bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

[Nicholas Vollmer]

Nicholas Vollmer <nv@parenthetic.dev> writes:

Attached is a proof-of-concept patch which addresses bugs #71404, 
#71405, and #71406 
I'll take care of that if/when the design is agreed upon.

The patch rewrites string-edit-mode as a minor mode.
It utilizes an overlay for the edit prompt (to persist across 
major mode changes) and prevents the prompt separator from being 
deleted.
It persists the buffer-local editing data so that the major mode 
can be changed and the edit can still be completed.
It uses #'ignore as the default abort function if none is 
provided.

bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

[Nicholas Vollmer]

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

Nicholas Vollmer <nv@parenthetic.dev> writes:

> Nicholas Vollmer <nv@parenthetic.dev> writes:
>
> Attached is a proof-of-concept patch which addresses bugs 
> #71404, #71405, and
> #71406 I'll take care of that if/when the design is agreed upon.
>
> The patch rewrites string-edit-mode as a minor mode.
> It utilizes an overlay for the edit prompt (to persist across 
> major mode
> changes) and prevents the prompt separator from being deleted.
> It persists the buffer-local editing data so that the major mode 
> can be changed
> and the edit can still be completed.
> It uses #'ignore as the default abort function if none is 
> provided.

Sorry. Forgot to attach patch.
Attached here.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-textmodes-string-edit.el-refactor-as-minor-mode.patch --]
[-- Type: text/x-patch, Size: 5458 bytes --]

From 10ae067b0590884c7df6854ca04a1863782c368c Mon Sep 17 00:00:00 2001
From: Nicholas Vollmer <iarchivedmywholelife@gmail.com>
Date: Fri, 7 Jun 2024 22:11:16 -0400
Subject: [PATCH] lisp/textmodes/string-edit.el: refactor as minor mode

---
 lisp/textmodes/string-edit.el | 82 +++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/lisp/textmodes/string-edit.el b/lisp/textmodes/string-edit.el
index 03be426ac25..0eeba50b8ed 100644
--- a/lisp/textmodes/string-edit.el
+++ b/lisp/textmodes/string-edit.el
@@ -23,20 +23,22 @@
 
 ;;; Code:
 
-(require 'cl-lib)
-
 (defface string-edit-prompt
   '((t (:inherit font-lock-comment-face)))
   "Face used on `string-edit' help text."
   :group 'text
   :version "29.1")
 
-(defvar string-edit--success-callback)
-(defvar string-edit--abort-callback)
+(defvar-local string-edit--success-callback nil)
+(put 'string-edit--success-callback 'permanent-local t)
+(defvar-local string-edit--abort-callback nil)
+(put 'string-edit--abort-callback 'permanent-local t)
+(defvar-local string-edit--prompt nil)
+(put 'string-edit--prompt 'permanent-local t)
+
 
 ;;;###autoload
-(cl-defun string-edit (prompt string success-callback
-                              &key abort-callback)
+(defun string-edit (prompt string success-callback &optional abort-callback)
   "Switch to a new buffer to edit STRING.
 When the user finishes editing (with \\<string-edit-mode-map>\\[string-edit-done]), SUCCESS-CALLBACK
 is called with the resulting string.
@@ -50,36 +52,16 @@ string-edit
 
 Also see `read-string-from-buffer'."
   (with-current-buffer (generate-new-buffer "*edit string*")
-    (when prompt
-      (let ((inhibit-read-only t))
-        (insert prompt)
-        (ensure-empty-lines 0)
-        (add-text-properties (point-min) (point)
-                             (list 'intangible t
-                                   'face 'string-edit-prompt
-                                   'read-only t))
-        (insert (propertize (make-separator-line) 'rear-nonsticky t))
-        (add-text-properties (point-min) (point)
-                             (list 'string-edit--prompt t))))
-    (let ((start (point)))
-      (insert string)
-      (goto-char start))
-
+    (setq string-edit--prompt prompt
+          string-edit--success-callback success-callback
+          string-edit--abort-callback (or abort-callback #'ignore))
+    (string-edit-mode)
+    (save-excursion (insert string))
     ;; Use `fit-window-to-buffer' after the buffer is filled with text.
     (pop-to-buffer (current-buffer)
                    '(display-buffer-below-selected
                      (window-height . (lambda (window)
                                         (fit-window-to-buffer window nil 10)))))
-
-    (set-buffer-modified-p nil)
-    (setq buffer-undo-list nil)
-    (string-edit-mode)
-    (setq-local string-edit--success-callback success-callback)
-    (when abort-callback
-      (setq-local string-edit--abort-callback abort-callback))
-    (setq-local header-line-format
-                (substitute-command-keys
-                 "Type \\<string-edit-mode-map>\\[string-edit-done] when you've finished editing or \\[string-edit-abort] to abort"))
     (message "%s" (substitute-command-keys
                    "Type \\<string-edit-mode-map>\\[string-edit-done] when you've finished editing"))))
 
@@ -99,9 +81,9 @@ read-string-from-buffer
    (lambda (edited)
      (setq string edited)
      (exit-recursive-edit))
-   :abort-callback (lambda ()
-                     (exit-recursive-edit)
-                     (error "Aborted edit")))
+   (lambda ()
+     (exit-recursive-edit)
+     (error "Aborted edit")))
   (recursive-edit)
   string)
 
@@ -109,9 +91,35 @@ string-edit-mode-map
   "C-c C-c" #'string-edit-done
   "C-c C-k" #'string-edit-abort)
 
-(define-derived-mode string-edit-mode text-mode "String"
-  "Mode for editing strings."
-  :interactive nil)
+(defun string-edit--prepare-buffer ()
+  "Prepare `string-edit-mode' buffer."
+  (with-silent-modifications
+    (when string-edit--prompt
+      (insert (propertize " " 'intangible t 'read-only t))
+      (let ((o (make-overlay (point-min) (point-max))))
+        (overlay-put o 'display
+                     (concat (propertize string-edit--prompt 'face 'string-edit-prompt)
+                             "\n"
+                             (propertize "\n" 0 1 'face '(:inherit separator-line :extend t))))
+        (overlay-put o 'evaporate nil)
+        (insert (propertize "\n" 'string-edit--prompt t 'read-only t 'rear-nonsticky t 'intangible t 'front-sticky t)))
+      (setq string-edit--prompt nil))
+    (setq header-line-format
+          (substitute-command-keys
+           "Type \\<string-edit-mode-map>\\[string-edit-done] when you've finished editing or \\[string-edit-abort] to abort"))
+    (setq buffer-undo-list nil)))
+
+(define-minor-mode string-edit-mode
+  "Minor mode for editing strings in a dedicated buffer."
+  :lighter " string-edit"
+  (cond
+   (string-edit-mode
+    (add-hook 'after-change-major-mode-hook #'string-edit-mode nil t)
+    (put 'after-change-major-mode-hook 'permanent-local t)
+    (string-edit--prepare-buffer))
+   (t
+    (remove-hook 'after-change-major-mode-hook #'string-edit-mode t)
+    (put 'after-change-major-mode-hook 'permanent-local nil))))
 
 (defun string-edit-done ()
   "Finish editing the string and call the callback function.
-- 
2.45.1

bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

[Nicholas Vollmer]

Nicholas Vollmer <nv@parenthetic.dev> writes:

> Nicholas Vollmer <nv@parenthetic.dev> writes:
>
>> Nicholas Vollmer <nv@parenthetic.dev> writes:
>>
>> Attached is a proof-of-concept patch which addresses bugs 
>> #71404, #71405, and
>> #71406 I'll take care of that if/when the design is agreed 
>> upon.
>>
>> The patch rewrites string-edit-mode as a minor mode.
>> It utilizes an overlay for the edit prompt (to persist across 
>> major mode
>> changes) and prevents the prompt separator from being deleted.
>> It persists the buffer-local editing data so that the major 
>> mode can be
>> changed
>> and the edit can still be completed.
>> It uses #'ignore as the default abort function if none is 
>> provided.
>
> Sorry. Forgot to attach patch.
> Attached here.
>
> [2. text/x-patch; 
> 0001-lisp-textmodes-string-edit.el-refactor-as-minor-mode.patch]...


Any thoughts here?

bug#71404: 30.0.50; [WISHLIST] string-edit-mode should allow changing major mode

[Eli Zaretskii]

> From: Nicholas Vollmer <nv@parenthetic.dev>
> CC: Eli Zaretskii <eliz@gnu.org>
> Date: Mon, 10 Jun 2024 16:53:47 -0400
> 
> Any thoughts here?

I didn't yet have time to look into this, sorry.