all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#70820: [PATCH] Editable grep buffers
@ 2024-05-07 16:25 Visuwesh
  2024-05-07 17:23 ` Jim Porter
  2024-05-07 18:08 ` Eli Zaretskii
  0 siblings, 2 replies; 34+ messages in thread
From: Visuwesh @ 2024-05-07 16:25 UTC (permalink / raw)
  To: 70820

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

Tags: patch

Attached patch is a proof of concept for an occur-edit-mode alike for
*grep* buffers.  It uses the new track-changes library to track the
edits made in the *grep* buffer to then finally save these edits to the
corresponding files.  I've tested this with:

    1. pure deletions: If you have a match that says "(cur-beg start"
       and you change it to "(cur- start".  This is special because
       track-changes-fetch passes equal BEG and END to
       grep-edit--track-changes-finalise.
    2. edits: something like "(cur-beg start" -> "(cur-beg start balh"
       is handled like you would expect.
    3. edits with newline: if the edit includes a newline, then that is
       reproduced.

and what is not handled currently: deleting a match line to imply
deletion of that line from the matched file.  I don't know how to handle
this currently, I need to learn the library more.

There's also definitely a million more cases that I didn't anticipate
when I wrote the logic for grep-edit--track-changes-finalise.

I'm sending the patch now to see if there's enough interest before I go
about fixing the edge cases (there's bug#52815).  If there is enough
interest, I will take a shot at implementing something like this for
xref result buffers too.



In GNU Emacs 30.0.50 (build 8, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.18.0, Xaw scroll bars) of 2024-05-07 built on astatine
Repository revision: 4b31074f5f49898ba0499a2b617cad425750eafa
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101011
System Description: Debian GNU/Linux trixie/sid

Configured using:
 'configure --with-sound=alsa --with-x-toolkit=lucid --with-json
 --without-xaw3d --without-gconf --without-libsystemd --with-cairo
 --with-xft'

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grep-edit.diff --]
[-- Type: text/patch, Size: 3654 bytes --]

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 657349cbdff..3fa6fce0e8d 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -31,6 +31,7 @@
 
 (eval-when-compile (require 'cl-lib))
 (require 'compile)
+(require 'track-changes)
 
 (defgroup grep nil
   "Run `grep' and display the results."
@@ -295,6 +296,8 @@ grep-mode-map
     (define-key map "}" #'compilation-next-file)
     (define-key map "\t" #'compilation-next-error)
     (define-key map [backtab] #'compilation-previous-error)
+
+    (define-key map "e" #'grep-edit-minor-mode)
     map)
   "Keymap for grep buffers.
 `compilation-minor-mode-map' is a cdr of this.")
@@ -1029,6 +1032,71 @@ grep
 		       command-args)
 		     #'grep-mode))
 
+(defvar-local grep-edit--tracker nil
+  "ID of the `track-changes' tracker.")
+
+(defun grep-edit--track-changes-signal (_id &optional _distance)
+  ;; Empty because we want to simply accumulate the changes at end
+  ;; when the user calls the finish function.
+  nil)
+
+(defun grep-edit--track-changes-finalise (beg end _before)
+  (let ((grep-buf (current-buffer))
+        (cur-res beg))                  ; Point at current grep result.
+    (save-excursion
+      (while (<= cur-res end)
+        (goto-char cur-res)
+        (let* ((change-beg (next-single-char-property-change (pos-bol) 'compilation-message))
+               ;; `1-' is required to ignore the newline.
+               (change-end (1- (next-single-char-property-change (pos-eol) 'compilation-message)))
+               (loc (compilation--message->loc
+                     (get-text-property (pos-bol) 'compilation-message)))
+               (line (compilation--loc->line loc))
+               (file (caar (compilation--loc->file-struct loc))))
+          (with-current-buffer (find-file-noselect file)
+            (save-excursion
+              (goto-char (point-min))
+              (forward-line (1- line))
+              (delete-region (pos-bol) (pos-eol))
+              (insert-buffer-substring-no-properties grep-buf change-beg change-end))))
+        (setq cur-res  (next-single-property-change cur-res 'compilation-message))))))
+
+(define-minor-mode grep-edit-minor-mode
+  "Minor mode for editing *grep* buffers.
+In this mode, changes to the *grep* buffer are applied to the
+originating files upon saving using \\[grep-save-changes]."
+  :lighter " Grep-Edit"
+  (if (null grep-edit-minor-mode)
+      (progn
+        (setq buffer-read-only t)
+        (buffer-disable-undo)
+        (use-local-map grep-mode-map))
+    (unless grep-edit--tracker
+      (use-local-map grep-edit-minor-mode-map)
+      (setq buffer-read-only nil)
+      (buffer-enable-undo)
+      (setq grep-edit--tracker
+            (track-changes-register #'grep-edit--track-changes-signal
+                                    :disjoint t)))
+    (message (substitute-command-keys
+              "Editing: \\[grep-edit-save-changes] to return to Grep mode."))))
+
+(defun grep-edit-save-changes ()
+  "Save the changes made to the *grep* buffer."
+  (interactive)
+  (when (and grep-edit-minor-mode grep-edit--tracker)
+    (track-changes-fetch grep-edit--tracker #'grep-edit--track-changes-finalise)
+    (message "Applied edits, switching to Grep mode.")
+    (track-changes-unregister grep-edit--tracker)
+    (setq grep-edit--tracker nil)
+    (grep-edit-minor-mode -1)))
+
+(defvar grep-edit-minor-mode-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map text-mode-map)
+    (define-key map (kbd "C-c C-c") #'grep-edit-save-changes)
+    map)
+  "Keymap for `grep-edit-minor-mode'.")
 
 ;;;###autoload
 (defun grep-find (command-args)

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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-07 16:25 bug#70820: [PATCH] Editable grep buffers Visuwesh
@ 2024-05-07 17:23 ` Jim Porter
  2024-05-08  3:12   ` Visuwesh
  2024-05-18 13:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-07 18:08 ` Eli Zaretskii
  1 sibling, 2 replies; 34+ messages in thread
From: Jim Porter @ 2024-05-07 17:23 UTC (permalink / raw)
  To: Visuwesh, 70820

On 5/7/2024 9:25 AM, Visuwesh wrote:
> I'm sending the patch now to see if there's enough interest before I go
> about fixing the edge cases (there's bug#52815).  If there is enough
> interest, I will take a shot at implementing something like this for
> xref result buffers too.

How does this compare to the existing wgrep package? That doesn't have 
copyright assignment to the FSF, but otherwise, I think it's the bar 
against which any similar package should be measured. I use it pretty 
frequently (and have even written a plugin for it for my own grep-like 
major mode), and it's pretty much exactly the way I'd want this sort of 
thing to work. Something with feature-parity that lives in the Emacs 
tree would be great though.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-07 16:25 bug#70820: [PATCH] Editable grep buffers Visuwesh
  2024-05-07 17:23 ` Jim Porter
@ 2024-05-07 18:08 ` Eli Zaretskii
  2024-05-08  3:22   ` Visuwesh
  1 sibling, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-07 18:08 UTC (permalink / raw)
  To: Visuwesh; +Cc: 70820

> From: Visuwesh <visuweshm@gmail.com>
> Date: Tue, 07 May 2024 21:55:09 +0530
> 
> Attached patch is a proof of concept for an occur-edit-mode alike for
> *grep* buffers.  It uses the new track-changes library to track the
> edits made in the *grep* buffer to then finally save these edits to the
> corresponding files.  I've tested this with:
> 
>     1. pure deletions: If you have a match that says "(cur-beg start"
>        and you change it to "(cur- start".  This is special because
>        track-changes-fetch passes equal BEG and END to
>        grep-edit--track-changes-finalise.
>     2. edits: something like "(cur-beg start" -> "(cur-beg start balh"
>        is handled like you would expect.
>     3. edits with newline: if the edit includes a newline, then that is
>        reproduced.
> 
> and what is not handled currently: deleting a match line to imply
> deletion of that line from the matched file.  I don't know how to handle
> this currently, I need to learn the library more.
> 
> There's also definitely a million more cases that I didn't anticipate
> when I wrote the logic for grep-edit--track-changes-finalise.
> 
> I'm sending the patch now to see if there's enough interest before I go
> about fixing the edge cases (there's bug#52815).  If there is enough
> interest, I will take a shot at implementing something like this for
> xref result buffers too.

Thanks for working on this.

I wonder if this could be somehow either based on or at least have the
same look-and-feel as occur-edit-mode, which provides a very similar
feature in *occur* buffers.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-07 17:23 ` Jim Porter
@ 2024-05-08  3:12   ` Visuwesh
  2024-05-08  4:11     ` Jim Porter
  2024-05-18 13:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 34+ messages in thread
From: Visuwesh @ 2024-05-08  3:12 UTC (permalink / raw)
  To: Jim Porter; +Cc: 70820

[செவ்வாய் மே 07, 2024] Jim Porter wrote:

> On 5/7/2024 9:25 AM, Visuwesh wrote:
>> I'm sending the patch now to see if there's enough interest before I go
>> about fixing the edge cases (there's bug#52815).  If there is enough
>> interest, I will take a shot at implementing something like this for
>> xref result buffers too.
>
> How does this compare to the existing wgrep package? That doesn't have
> copyright assignment to the FSF, but otherwise, I think it's the bar
> against which any similar package should be measured. I use it pretty
> frequently (and have even written a plugin for it for my own grep-like
> major mode), and it's pretty much exactly the way I'd want this sort
> of thing to work. Something with feature-parity that lives in the
> Emacs tree would be great though.

I definitely agree that wgrep is the standard to which my patch should
be compared with.  I haven't actually used wgrep before but I took a
look at its README and so far, the features that are missing are:

    · I already quoted: deleting lines from the file.  I may tackle this
      once I get the basic editing features reliable.
    · Aborting changes wholesale: this should be easy and is already on
      my list.
    · Aborting changes in a region: I wonder if we cannot simply use
      undo restricted to a region to do it.  But otherwise, I can take a
      look at implementing this since I am also interested in this (but
      once I get the basics reliable).

Thanks for your input.  If I missed something from wgrep that you use,
please let me known.  I will play around with wgrep in some time to
learn more about what it offers myself.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-07 18:08 ` Eli Zaretskii
@ 2024-05-08  3:22   ` Visuwesh
  2024-05-08 11:58     ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Visuwesh @ 2024-05-08  3:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820

[செவ்வாய் மே 07, 2024] Eli Zaretskii wrote:

> [...]
> Thanks for working on this.
>
> I wonder if this could be somehow either based on 

Basing it on occur-edit-mode would be a lot more work I think, but I
understand your concern wrt it being already established and bug-free,
etc.  This was my original plan but I bailed since the occur buffer's
text-properties has marker objects (IIRC) but I want to avoid using
markers since having many buffers open was a personal pet peeve of mine,
along with the slow-typing experience due to occur's
after-change-function immediately reflecting the changes in the original
buffer.  The latter is avoided in my patch since we commit the changes
only at the end so the typing during the edit is smooth.

> or at least have the
> same look-and-feel as occur-edit-mode, which provides a very similar
> feature in *occur* buffers.

This is what I am aiming for ideally since I am a fan of its interface
myself (preferring it over xref).  The major difference between
occur-edit-mode and my patch's grep-edit-minor-mode is that it is
implemented as a minor mode: I chose a minor-mode because all the buffer
local variables that are required to make the commands from the *grep*
buffer functional are killed when switching major-modes (so even g
doesn't work!). We could work around this by marking the relevant
variables permanent-local but this feels unclean?

The other difference that I can see now: the filename and the line
number parts aren't marked read-only so it can be edited when
grep-edit-minor-mode is active.  I will mark them read-only to have the
experience closer to occur-edit-mode.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08  3:12   ` Visuwesh
@ 2024-05-08  4:11     ` Jim Porter
  2024-05-08  5:11       ` Visuwesh
  0 siblings, 1 reply; 34+ messages in thread
From: Jim Porter @ 2024-05-08  4:11 UTC (permalink / raw)
  To: Visuwesh; +Cc: 70820

On 5/7/2024 8:12 PM, Visuwesh wrote:
> Thanks for your input.  If I missed something from wgrep that you use,
> please let me known.  I will play around with wgrep in some time to
> learn more about what it offers myself.

In addition to the things you mentioned, here are the most important 
features from wgrep for me:

* Mark all non-result parts of the buffer (the command string, file 
names, line numbers, etc) as read-only. This is especially valuable if 
you want to use 'query-replace' or similar to modify the results. Then 
you can't inadvertently edit those bits.

* Fontify any results with changes (both in the grep-mode buffer and the 
original files). This is really useful for being able to see what I've 
changed.

* Adding all the necessary hooks/functions so other grep-like modes can 
use this. For example, see this file from wgrep, which lets you use 
wgrep with ag.el: 
<https://github.com/mhayashi1120/Emacs-wgrep/blob/master/wgrep-ag.el>. 
(This matters to me partly because I'm the author of Urgrep - a 
"universal recursive grep" mode that can use any grep-like program to do 
searches. I've added my own support in Urgrep for wgrep.)





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08  4:11     ` Jim Porter
@ 2024-05-08  5:11       ` Visuwesh
  0 siblings, 0 replies; 34+ messages in thread
From: Visuwesh @ 2024-05-08  5:11 UTC (permalink / raw)
  To: Jim Porter; +Cc: 70820

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

[செவ்வாய் மே 07, 2024] Jim Porter wrote:

> On 5/7/2024 8:12 PM, Visuwesh wrote:
>> Thanks for your input.  If I missed something from wgrep that you use,
>> please let me known.  I will play around with wgrep in some time to
>> learn more about what it offers myself.
>
> In addition to the things you mentioned, here are the most important
> features from wgrep for me:

Thanks.

> * Mark all non-result parts of the buffer (the command string, file
>   names, line numbers, etc) as read-only. This is especially valuable
>   if you want to use 'query-replace' or similar to modify the
>   results. Then you can't inadvertently edit those bits.

This is now done.

> * Fontify any results with changes (both in the grep-mode buffer and
>   the original files). This is really useful for being able to see
>   what I've changed.

I agree that it would be nice to have this in *grep* buffer but I would
like to avoid editing the file on-the-fly since it introduces typing lag
IME with occur-edit-mode.  If you want to know the edits that were made,
you can use diff-buffer-with-file so I hope leaving out the highlighting
in the file would be okay.

> * Adding all the necessary hooks/functions so other grep-like modes
>   can use this. For example, see this file from wgrep, which lets you
>   use wgrep with ag.el:
>   <https://github.com/mhayashi1120/Emacs-wgrep/blob/master/wgrep-ag.el>. (This
>   matters to me partly because I'm the author of Urgrep - a "universal
>   recursive grep" mode that can use any grep-like program to do
>   searches. I've added my own support in Urgrep for wgrep.)

With grep-edit-minor-mode, as long as the compilation-message
text-property is present at the beginning of the line, you do not need
to do anything extra.  I look at this to gain the information about the
filename and the line number.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grep-edit.diff --]
[-- Type: text/x-diff, Size: 4328 bytes --]

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 657349cbdff..517d1bd5ecd 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -31,6 +31,7 @@
 
 (eval-when-compile (require 'cl-lib))
 (require 'compile)
+(require 'track-changes)
 
 (defgroup grep nil
   "Run `grep' and display the results."
@@ -295,6 +296,8 @@ grep-mode-map
     (define-key map "}" #'compilation-next-file)
     (define-key map "\t" #'compilation-next-error)
     (define-key map [backtab] #'compilation-previous-error)
+
+    (define-key map "e" #'grep-edit-minor-mode)
     map)
   "Keymap for grep buffers.
 `compilation-minor-mode-map' is a cdr of this.")
@@ -1029,6 +1032,86 @@ grep
 		       command-args)
 		     #'grep-mode))
 
+(defvar-local grep-edit--tracker nil
+  "ID of the `track-changes' tracker.")
+
+(defun grep-edit--track-changes-signal (_id &optional _distance)
+  ;; Empty because we want to simply accumulate the changes at end
+  ;; when the user calls the finish function.
+  nil)
+
+(defun grep-edit--track-changes-finalise (beg end _before)
+  (let ((grep-buf (current-buffer))
+        (cur-res beg))                  ; Point at current grep result.
+    (save-excursion
+      (while (<= cur-res end)
+        (goto-char cur-res)
+        (let* ((change-beg (next-single-char-property-change (pos-bol) 'compilation-message))
+               ;; `1-' is required to ignore the newline.
+               (change-end (1- (next-single-char-property-change (pos-eol) 'compilation-message)))
+               (loc (compilation--message->loc
+                     (get-text-property (pos-bol) 'compilation-message)))
+               (line (compilation--loc->line loc))
+               (file (caar (compilation--loc->file-struct loc))))
+          (with-current-buffer (find-file-noselect file)
+            (save-excursion
+              (goto-char (point-min))
+              (forward-line (1- line))
+              (delete-region (pos-bol) (pos-eol))
+              (insert-buffer-substring-no-properties grep-buf change-beg change-end))))
+        (setq cur-res  (next-single-property-change cur-res 'compilation-message))))))
+
+(defun grep-edit--mark-read-only ()
+  "Mark annotations and info regions read-only."
+  (save-excursion
+    (goto-char (point-min))
+    (let ((inhibit-read-only t)
+          match)
+      (while (setq match (text-property-search-forward 'compilation-annotation))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t)))
+      (goto-char (point-min))
+      (while (setq match (text-property-search-forward 'compilation-message))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t))))))
+
+(define-minor-mode grep-edit-minor-mode
+  "Minor mode for editing *grep* buffers.
+In this mode, changes to the *grep* buffer are applied to the
+originating files upon saving using \\[grep-save-changes]."
+  :lighter " Grep-Edit"
+  (if (null grep-edit-minor-mode)
+      (progn
+        (setq buffer-read-only t)
+        (buffer-disable-undo)
+        (use-local-map grep-mode-map))
+    (grep-edit--mark-read-only)
+    (unless grep-edit--tracker
+      (use-local-map grep-edit-minor-mode-map)
+      (setq buffer-read-only nil)
+      (buffer-enable-undo)
+      (setq grep-edit--tracker
+            (track-changes-register #'grep-edit--track-changes-signal
+                                    :disjoint t)))
+    (message (substitute-command-keys
+              "Editing: \\[grep-edit-save-changes] to return to Grep mode."))))
+
+(defun grep-edit-save-changes ()
+  "Save the changes made to the *grep* buffer."
+  (interactive)
+  (when (and grep-edit-minor-mode grep-edit--tracker)
+    (track-changes-fetch grep-edit--tracker #'grep-edit--track-changes-finalise)
+    (message "Applied edits, switching to Grep mode.")
+    (track-changes-unregister grep-edit--tracker)
+    (setq grep-edit--tracker nil)
+    (grep-edit-minor-mode -1)))
+
+(defvar grep-edit-minor-mode-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map text-mode-map)
+    (define-key map (kbd "C-c C-c") #'grep-edit-save-changes)
+    map)
+  "Keymap for `grep-edit-minor-mode'.")
 
 ;;;###autoload
 (defun grep-find (command-args)

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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08  3:22   ` Visuwesh
@ 2024-05-08 11:58     ` Eli Zaretskii
  2024-05-08 12:18       ` Visuwesh
  2024-05-08 17:37       ` Jim Porter
  0 siblings, 2 replies; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-08 11:58 UTC (permalink / raw)
  To: Visuwesh; +Cc: 70820

> From: Visuwesh <visuweshm@gmail.com>
> Cc: 70820@debbugs.gnu.org
> Date: Wed, 08 May 2024 08:52:51 +0530
> 
> [செவ்வாய் மே 07, 2024] Eli Zaretskii wrote:
> 
> > [...]
> > Thanks for working on this.
> >
> > I wonder if this could be somehow either based on 
> 
> Basing it on occur-edit-mode would be a lot more work I think, but I
> understand your concern wrt it being already established and bug-free,
> etc.  This was my original plan but I bailed since the occur buffer's
> text-properties has marker objects (IIRC) but I want to avoid using
> markers since having many buffers open was a personal pet peeve of mine,
> along with the slow-typing experience due to occur's
> after-change-function immediately reflecting the changes in the original
> buffer.  The latter is avoided in my patch since we commit the changes
> only at the end so the typing during the edit is smooth.

I think having similar features that work very differently is not a
good thing for Emacs.  So I urge you to reconsider your decisions and
make this more like occur-edit-mode.  In particular, I don't
understand the difficulty with using the markers and what does it have
to do with the ability of having many Grep buffers.

> This is what I am aiming for ideally since I am a fan of its interface
> myself (preferring it over xref).  The major difference between
> occur-edit-mode and my patch's grep-edit-minor-mode is that it is
> implemented as a minor mode: I chose a minor-mode because all the buffer
> local variables that are required to make the commands from the *grep*
> buffer functional are killed when switching major-modes (so even g
> doesn't work!). We could work around this by marking the relevant
> variables permanent-local but this feels unclean?

I don't think I understand this difficulty, either: with
occur-edit-mode it is solved by making occur-edit-mode be derived from
occur-mode.  Couldn't you do the same with your mode?





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 11:58     ` Eli Zaretskii
@ 2024-05-08 12:18       ` Visuwesh
  2024-05-08 13:49         ` Eli Zaretskii
  2024-05-08 17:37       ` Jim Porter
  1 sibling, 1 reply; 34+ messages in thread
From: Visuwesh @ 2024-05-08 12:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820

[புதன் மே 08, 2024] Eli Zaretskii wrote:

>> From: Visuwesh <visuweshm@gmail.com>
>> Cc: 70820@debbugs.gnu.org
>> Date: Wed, 08 May 2024 08:52:51 +0530
>> 
>> [செவ்வாய் மே 07, 2024] Eli Zaretskii wrote:
>> 
>> > [...]
>> > Thanks for working on this.
>> >
>> > I wonder if this could be somehow either based on 
>> 
>> Basing it on occur-edit-mode would be a lot more work I think, but I
>> understand your concern wrt it being already established and bug-free,
>> etc.  This was my original plan but I bailed since the occur buffer's
>> text-properties has marker objects (IIRC) but I want to avoid using
>> markers since having many buffers open was a personal pet peeve of mine,
>> along with the slow-typing experience due to occur's
>> after-change-function immediately reflecting the changes in the original
>> buffer.  The latter is avoided in my patch since we commit the changes
>> only at the end so the typing during the edit is smooth.
>
> I think having similar features that work very differently is not a
> good thing for Emacs.  So I urge you to reconsider your decisions and
> make this more like occur-edit-mode.  In particular, I don't
> understand the difficulty with using the markers and what does it have
> to do with the ability of having many Grep buffers.

It is not having many Grep buffers but visiting the "original" files
unnecessarily that tends to be annoying.  I am not sure if there is a
scheme to make these visits conservative, I would need to look further.

I do not think using track-changes to keep track of the edits will do
too much harm since the trickiest part of keeping track of the buffer
modifications proper is left to an "external" library.  If I'm not
wrong, track-changes was introduced to make looking at buffer
modifications less error-prone.

>> This is what I am aiming for ideally since I am a fan of its interface
>> myself (preferring it over xref).  The major difference between
>> occur-edit-mode and my patch's grep-edit-minor-mode is that it is
>> implemented as a minor mode: I chose a minor-mode because all the buffer
>> local variables that are required to make the commands from the *grep*
>> buffer functional are killed when switching major-modes (so even g
>> doesn't work!). We could work around this by marking the relevant
>> variables permanent-local but this feels unclean?
>
> I don't think I understand this difficulty, either: with
> occur-edit-mode it is solved by making occur-edit-mode be derived from
> occur-mode.  Couldn't you do the same with your mode?

No because occur-mode makes occur-revert-arguments permanent-local so
`g' survives the major-mode changes.

For revert-buffer alone, compilation-arguments needs to be marked
permanent-local.  As it is a part of compile.el, I am not sure if
marking it as such is safe.  This is why I think having a minor-mode is
better.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 12:18       ` Visuwesh
@ 2024-05-08 13:49         ` Eli Zaretskii
  2024-05-09 10:32           ` Visuwesh
  2024-05-18 13:34           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-08 13:49 UTC (permalink / raw)
  To: Visuwesh; +Cc: 70820

> From: Visuwesh <visuweshm@gmail.com>
> Cc: 70820@debbugs.gnu.org
> Date: Wed, 08 May 2024 17:48:02 +0530
> 
> [புதன் மே 08, 2024] Eli Zaretskii wrote:
> 
> > I think having similar features that work very differently is not a
> > good thing for Emacs.  So I urge you to reconsider your decisions and
> > make this more like occur-edit-mode.  In particular, I don't
> > understand the difficulty with using the markers and what does it have
> > to do with the ability of having many Grep buffers.
> 
> It is not having many Grep buffers but visiting the "original" files
> unnecessarily that tends to be annoying.

Why is this annoying?

> > I don't think I understand this difficulty, either: with
> > occur-edit-mode it is solved by making occur-edit-mode be derived from
> > occur-mode.  Couldn't you do the same with your mode?
> 
> No because occur-mode makes occur-revert-arguments permanent-local so
> `g' survives the major-mode changes.
> 
> For revert-buffer alone, compilation-arguments needs to be marked
> permanent-local.  As it is a part of compile.el, I am not sure if
> marking it as such is safe.  This is why I think having a minor-mode is
> better.

It sounds like a minor issue which shouldn't have such grave
consequences.  Why do you think making compilation-arguments
permanent-local would be a problem?  We could ask people who
frequently contribute to compile.e land grep.el if they see any
problem with doing that.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 11:58     ` Eli Zaretskii
  2024-05-08 12:18       ` Visuwesh
@ 2024-05-08 17:37       ` Jim Porter
  2024-05-08 18:42         ` Eli Zaretskii
  1 sibling, 1 reply; 34+ messages in thread
From: Jim Porter @ 2024-05-08 17:37 UTC (permalink / raw)
  To: Eli Zaretskii, Visuwesh; +Cc: 70820

On 5/8/2024 4:58 AM, Eli Zaretskii wrote:
>> From: Visuwesh <visuweshm@gmail.com>
>> Cc: 70820@debbugs.gnu.org
>> Date: Wed, 08 May 2024 08:52:51 +0530
>>
>> Basing it on occur-edit-mode would be a lot more work I think, but I
>> understand your concern wrt it being already established and bug-free,
>> etc.  This was my original plan but I bailed since the occur buffer's
>> text-properties has marker objects (IIRC) but I want to avoid using
>> markers since having many buffers open was a personal pet peeve of mine,
>> along with the slow-typing experience due to occur's
>> after-change-function immediately reflecting the changes in the original
>> buffer.  The latter is avoided in my patch since we commit the changes
>> only at the end so the typing during the edit is smooth.
> 
> I think having similar features that work very differently is not a
> good thing for Emacs.  So I urge you to reconsider your decisions and
> make this more like occur-edit-mode.  In particular, I don't
> understand the difficulty with using the markers and what does it have
> to do with the ability of having many Grep buffers.

I agree that using markers would probably be a good idea, assuming I'm 
imagining things correctly. (In particular, this needs to be robust 
about what happens if you have a file open with some changes already, 
run grep to find matches in that file, and then modify those matches.)

However, I agree with Visuwesh about not committing changes until the 
end. For the grep case, you could have results in many, many files, 
including (especially?) ones not open in Emacs yet. By waiting until the 
end to commit the changes, you don't have to worry about what happen to 
these files. (The only other options I can think of would be to visit 
those files, which could require opening hundreds or thousands of files 
at once; or to immediately change the files on disk, which could ruin 
those files.)





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 17:37       ` Jim Porter
@ 2024-05-08 18:42         ` Eli Zaretskii
  2024-05-08 19:19           ` Jim Porter
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-08 18:42 UTC (permalink / raw)
  To: Jim Porter; +Cc: 70820, visuweshm

> Date: Wed, 8 May 2024 10:37:42 -0700
> Cc: 70820@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> However, I agree with Visuwesh about not committing changes until the 
> end. For the grep case, you could have results in many, many files, 
> including (especially?) ones not open in Emacs yet.

I don't see why.  Grep shows all the matches in one file before it
goes to the next.  So each time you edit the matches, you have just
one file to care about, right?  Or what am I missing?





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 18:42         ` Eli Zaretskii
@ 2024-05-08 19:19           ` Jim Porter
  2024-05-08 19:23             ` Jim Porter
  0 siblings, 1 reply; 34+ messages in thread
From: Jim Porter @ 2024-05-08 19:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, visuweshm

On 5/8/2024 11:42 AM, Eli Zaretskii wrote:
>> Date: Wed, 8 May 2024 10:37:42 -0700
>> Cc: 70820@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> However, I agree with Visuwesh about not committing changes until the
>> end. For the grep case, you could have results in many, many files,
>> including (especially?) ones not open in Emacs yet.
> 
> I don't see why.  Grep shows all the matches in one file before it
> goes to the next.  So each time you edit the matches, you have just
> one file to care about, right?  Or what am I missing?

One of the ways I use wgrep is that I first search across my entire repo 
for some matching lines. Then, I activate wgrep and use query-replace to 
mass-change all those lines. (I use the "!" key with query-replace to 
tell it to change everything without prompting once I'm happy with my 
command.) This frequently involves me changing dozens of files at once 
even for relatively small projects.

Maybe there are better ways to perform this sort of action, but I find 
this method very useful because it shows me the steps along the way so 
that I can check my work before I commit to changing all those lines. I 
also don't have to manually step through each change once I'm happy with 
what I see.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 19:19           ` Jim Porter
@ 2024-05-08 19:23             ` Jim Porter
  2024-05-09  4:41               ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Jim Porter @ 2024-05-08 19:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, visuweshm

On 5/8/2024 12:19 PM, Jim Porter wrote:
> Maybe there are better ways to perform this sort of action, but I find 
> this method very useful because it shows me the steps along the way so 
> that I can check my work before I commit to changing all those lines. I 
> also don't have to manually step through each change once I'm happy with 
> what I see.

Oh, one thing I forgot to add about this: If I perform this action and 
the changed lines in my grep buffer don't look right, I can just exit 
wgrep without saving the changes and try again.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 19:23             ` Jim Porter
@ 2024-05-09  4:41               ` Eli Zaretskii
  2024-05-09 16:14                 ` Jim Porter
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-09  4:41 UTC (permalink / raw)
  To: Jim Porter; +Cc: 70820, visuweshm

> Date: Wed, 8 May 2024 12:23:00 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> Cc: 70820@debbugs.gnu.org, visuweshm@gmail.com
> 
> On 5/8/2024 12:19 PM, Jim Porter wrote:
> > Maybe there are better ways to perform this sort of action, but I find 
> > this method very useful because it shows me the steps along the way so 
> > that I can check my work before I commit to changing all those lines. I 
> > also don't have to manually step through each change once I'm happy with 
> > what I see.
> 
> Oh, one thing I forgot to add about this: If I perform this action and 
> the changed lines in my grep buffer don't look right, I can just exit 
> wgrep without saving the changes and try again.

We are not going to remove wgrep, so if you prefer that way of
operating on grep hits, you still can.  Here we are discussing a new
feature, and it doesn't have to be bit-by-bit compatible to all the
similar packages out there.  But it does need to make sense as part of
Emacs, if it is part of grep.el.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 13:49         ` Eli Zaretskii
@ 2024-05-09 10:32           ` Visuwesh
  2024-05-12  4:45             ` Visuwesh
  2024-05-18 13:34           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 34+ messages in thread
From: Visuwesh @ 2024-05-09 10:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820

[புதன் மே 08, 2024] Eli Zaretskii wrote:

>> From: Visuwesh <visuweshm@gmail.com>
>> Cc: 70820@debbugs.gnu.org
>> Date: Wed, 08 May 2024 17:48:02 +0530
>> 
>> [புதன் மே 08, 2024] Eli Zaretskii wrote:
>> 
>> > I think having similar features that work very differently is not a
>> > good thing for Emacs.  So I urge you to reconsider your decisions and
>> > make this more like occur-edit-mode.  In particular, I don't
>> > understand the difficulty with using the markers and what does it have
>> > to do with the ability of having many Grep buffers.
>> 
>> It is not having many Grep buffers but visiting the "original" files
>> unnecessarily that tends to be annoying.
>
> Why is this annoying?
>
>> > I don't think I understand this difficulty, either: with
>> > occur-edit-mode it is solved by making occur-edit-mode be derived from
>> > occur-mode.  Couldn't you do the same with your mode?
>> 
>> No because occur-mode makes occur-revert-arguments permanent-local so
>> `g' survives the major-mode changes.
>> 
>> For revert-buffer alone, compilation-arguments needs to be marked
>> permanent-local.  As it is a part of compile.el, I am not sure if
>> marking it as such is safe.  This is why I think having a minor-mode is
>> better.
>
> It sounds like a minor issue which shouldn't have such grave
> consequences.  Why do you think making compilation-arguments
> permanent-local would be a problem?  We could ask people who
> frequently contribute to compile.e land grep.el if they see any
> problem with doing that.

It feels like a potential far-fetching change.  But in any case, I will
give a shot at recreating the required markers, etc. for
occur-after-change-function to work.  Will send a patch once I have a
working prototype.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-09  4:41               ` Eli Zaretskii
@ 2024-05-09 16:14                 ` Jim Porter
  0 siblings, 0 replies; 34+ messages in thread
From: Jim Porter @ 2024-05-09 16:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, visuweshm

On 5/8/2024 9:41 PM, Eli Zaretskii wrote:
> We are not going to remove wgrep, so if you prefer that way of
> operating on grep hits, you still can.  Here we are discussing a new
> feature, and it doesn't have to be bit-by-bit compatible to all the
> similar packages out there.  But it does need to make sense as part of
> Emacs, if it is part of grep.el.

I certainly don't expect this facility to be identical to wgrep (I 
didn't bother mentioning the bits of wgrep that I'm not interested in), 
but I think it does have some very good ideas too.

For the case of only making changes at the end, I mainly wanted to 
express my agreement with Visuwesh's initial implementation and to 
explain how wgrep's similar behavior makes certain tasks easier.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-09 10:32           ` Visuwesh
@ 2024-05-12  4:45             ` Visuwesh
  2024-05-18  9:28               ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Visuwesh @ 2024-05-12  4:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820

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

[வியாழன் மே 09, 2024] Visuwesh wrote:

>>> > I don't think I understand this difficulty, either: with
>>> > occur-edit-mode it is solved by making occur-edit-mode be derived from
>>> > occur-mode.  Couldn't you do the same with your mode?
>>> 
>>> No because occur-mode makes occur-revert-arguments permanent-local so
>>> `g' survives the major-mode changes.
>>> 
>>> For revert-buffer alone, compilation-arguments needs to be marked
>>> permanent-local.  As it is a part of compile.el, I am not sure if
>>> marking it as such is safe.  This is why I think having a minor-mode is
>>> better.
>>
>> It sounds like a minor issue which shouldn't have such grave
>> consequences.  Why do you think making compilation-arguments
>> permanent-local would be a problem?  We could ask people who
>> frequently contribute to compile.e land grep.el if they see any
>> problem with doing that.
>
> It feels like a potential far-fetching change.  But in any case, I will
> give a shot at recreating the required markers, etc. for
> occur-after-change-function to work.  Will send a patch once I have a
> working prototype.

OK, please find attached patch.  I am still not using a major-mode
because of the change-major-mode-hook in compilation-setup:

    (add-hook 'change-major-mode-hook #'compilation--remove-properties nil t)

that strips off all the text-properties.  And there seem to be too many
important local variables that is set up by both grep.el and compile.el
so I think it is easier to use a minor-mode here.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grep-edit.diff --]
[-- Type: text/x-diff, Size: 9792 bytes --]

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index b18eb81fee1..867ebc92d75 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2829,6 +2829,53 @@ compilation-find-buffer
       (current-buffer)
     (next-error-find-buffer avoid-current 'compilation-buffer-internal-p)))
 
+(defun compilation--update-markers (loc marker screen-columns first-column)
+  "Update markers in LOC, and set MARKER to location pointed by LOC.
+SCREEN-COLUMNS and FIRST-COLUMN are the value of
+`compilation-error-screen-columns' and `compilation-first-column' to use
+if they are not set buffer-locally in the target buffer."
+  (with-current-buffer
+      (if (bufferp (caar (compilation--loc->file-struct loc)))
+          (caar (compilation--loc->file-struct loc))
+        (apply #'compilation-find-file
+               marker
+               (caar (compilation--loc->file-struct loc))
+               (cadr (car (compilation--loc->file-struct loc)))
+               (compilation--file-struct->formats
+                (compilation--loc->file-struct loc))))
+    (let ((screen-columns
+           ;; Obey the compilation-error-screen-columns of the target
+           ;; buffer if its major mode set it buffer-locally.
+           (if (local-variable-p 'compilation-error-screen-columns)
+               compilation-error-screen-columns screen-columns))
+          (compilation-first-column
+           (if (local-variable-p 'compilation-first-column)
+               compilation-first-column first-column))
+          (last 1))
+      (save-restriction
+        (widen)
+        (goto-char (point-min))
+        ;; Treat file's found lines in forward order, 1 by 1.
+        (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
+          (when (car line)     ; else this is a filename without a line#
+            (compilation-beginning-of-line (- (car line) last -1))
+            (setq last (car line)))
+          ;; Treat line's found columns and store/update a marker for each.
+          (dolist (col (cdr line))
+            (if (compilation--loc->col col)
+                (if (eq (compilation--loc->col col) -1)
+                    ;; Special case for range end.
+                    (end-of-line)
+                  (compilation-move-to-column (compilation--loc->col col)
+                                              screen-columns))
+              (beginning-of-line)
+              (skip-chars-forward " \t"))
+            (if (compilation--loc->marker col)
+                (set-marker (compilation--loc->marker col) (point))
+              (setf (compilation--loc->marker col) (point-marker)))
+            ;; (setf (compilation--loc->timestamp col) timestamp)
+            ))))))
+
 ;;;###autoload
 (defun compilation-next-error-function (n &optional reset)
   "Advance to the next error message and visit the file where the error was.
@@ -2838,7 +2885,6 @@ compilation-next-error-function
     (setq compilation-current-error nil))
   (let* ((screen-columns compilation-error-screen-columns)
 	 (first-column compilation-first-column)
-	 (last 1)
 	 (msg (compilation-next-error (or n 1) nil
 				      (or compilation-current-error
 					  compilation-messages-start
@@ -2850,9 +2896,9 @@ compilation-next-error-function
       (user-error "No next error"))
     (setq compilation-current-error (point-marker)
 	  overlay-arrow-position
-	    (if (bolp)
-		compilation-current-error
-	      (copy-marker (line-beginning-position))))
+	  (if (bolp)
+	      compilation-current-error
+	    (copy-marker (line-beginning-position))))
     ;; If loc contains no marker, no error in that file has been visited.
     ;; If the marker is invalid the buffer has been killed.
     ;; So, recalculate all markers for that file.
@@ -2869,46 +2915,7 @@ compilation-next-error-function
                  ;;     (equal (compilation--loc->timestamp loc)
                  ;;            (setq timestamp compilation-buffer-modtime)))
                  )
-      (with-current-buffer
-          (if (bufferp (caar (compilation--loc->file-struct loc)))
-              (caar (compilation--loc->file-struct loc))
-            (apply #'compilation-find-file
-                   marker
-                   (caar (compilation--loc->file-struct loc))
-                   (cadr (car (compilation--loc->file-struct loc)))
-                   (compilation--file-struct->formats
-                    (compilation--loc->file-struct loc))))
-        (let ((screen-columns
-               ;; Obey the compilation-error-screen-columns of the target
-               ;; buffer if its major mode set it buffer-locally.
-               (if (local-variable-p 'compilation-error-screen-columns)
-                   compilation-error-screen-columns screen-columns))
-              (compilation-first-column
-               (if (local-variable-p 'compilation-first-column)
-                   compilation-first-column first-column)))
-          (save-restriction
-            (widen)
-            (goto-char (point-min))
-            ;; Treat file's found lines in forward order, 1 by 1.
-            (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
-              (when (car line)		; else this is a filename without a line#
-                (compilation-beginning-of-line (- (car line) last -1))
-                (setq last (car line)))
-              ;; Treat line's found columns and store/update a marker for each.
-              (dolist (col (cdr line))
-                (if (compilation--loc->col col)
-                    (if (eq (compilation--loc->col col) -1)
-                        ;; Special case for range end.
-                        (end-of-line)
-                      (compilation-move-to-column (compilation--loc->col col)
-                                                  screen-columns))
-                  (beginning-of-line)
-                  (skip-chars-forward " \t"))
-                (if (compilation--loc->marker col)
-                    (set-marker (compilation--loc->marker col) (point))
-                  (setf (compilation--loc->marker col) (point-marker)))
-                ;; (setf (compilation--loc->timestamp col) timestamp)
-                ))))))
+      (compilation--update-markers loc marker screen-columns first-column))
     (compilation-goto-locus marker (compilation--loc->marker loc)
                             (compilation--loc->marker end-loc))
     (setf (compilation--loc->visited loc) t)))
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 657349cbdff..1b1de0ef77d 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -295,6 +295,8 @@ grep-mode-map
     (define-key map "}" #'compilation-next-file)
     (define-key map "\t" #'compilation-next-error)
     (define-key map [backtab] #'compilation-previous-error)
+
+    (define-key map "e" #'grep-edit-minor-mode)
     map)
   "Keymap for grep buffers.
 `compilation-minor-mode-map' is a cdr of this.")
@@ -1029,6 +1031,66 @@ grep
 		       command-args)
 		     #'grep-mode))
 
+(defun grep-edit--prepare-buffer ()
+  "Mark relevant regions read-only, and add relevant occur text-properties."
+  (save-excursion
+    (goto-char (point-min))
+    (let ((inhibit-read-only t)
+          (dummy (make-marker))
+          match)
+      (while (setq match (text-property-search-forward 'compilation-annotation))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t)))
+      (goto-char (point-min))
+      (while (setq match (text-property-search-forward 'compilation-message))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t occur-prefix t))
+        (let ((loc (compilation--message->loc (prop-match-value match)))
+              m)
+          ;; Update the markers if necessary.
+          (unless (and (compilation--loc->marker loc)
+                       (marker-buffer (compilation--loc->marker loc)))
+            (compilation--update-markers loc dummy compilation-error-screen-columns compilation-first-column))
+          (setq m (compilation--loc->marker loc))
+          (add-text-properties (prop-match-beginning match)
+                               (or (next-single-property-change
+                                    (prop-match-end match)
+                                    'compilation-message)
+                                   (1+ (pos-eol)))
+                               `(occur-target ((,m . ,m)))))))))
+
+(defvar grep-edit-minor-mode-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map text-mode-map)
+    (define-key map (kbd "C-c C-c") #'grep-edit-save-changes)
+    map)
+  "Keymap for `grep-edit-minor-mode'.")
+
+(define-minor-mode grep-edit-minor-mode
+  "Minor mode for editing *grep* buffers.
+In this mode, changes to the *grep* buffer are applied to the
+originating files."
+  :lighter " Grep-Edit"
+  (if (null grep-edit-minor-mode)
+      (progn
+        (setq buffer-read-only t)
+        (buffer-disable-undo)
+        (remove-hook 'after-change-functions #'occur-after-change-function t)
+        (use-local-map grep-mode-map))
+    (grep-edit--prepare-buffer)
+    (use-local-map grep-edit-minor-mode-map)
+    (setq buffer-read-only nil)
+    (buffer-enable-undo)
+    (add-hook 'after-change-functions #'occur-after-change-function nil t)
+    (message (substitute-command-keys
+              "Editing: \\[grep-edit-save-changes] to return to Grep mode."))))
+
+(defun grep-edit-save-changes ()
+  "Switch back to Grep mode."
+  (interactive)
+  (when grep-edit-minor-mode
+    (message "Switching to Grep mode.")
+    (grep-edit-minor-mode -1)))
 
 ;;;###autoload
 (defun grep-find (command-args)

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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-12  4:45             ` Visuwesh
@ 2024-05-18  9:28               ` Eli Zaretskii
  2024-05-18 13:35                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-18  9:28 UTC (permalink / raw)
  To: Visuwesh, Stefan Monnier; +Cc: 70820

> From: Visuwesh <visuweshm@gmail.com>
> Cc: 70820@debbugs.gnu.org
> Date: Sun, 12 May 2024 10:15:33 +0530
> 
> [வியாழன் மே 09, 2024] Visuwesh wrote:
> 
> >>> > I don't think I understand this difficulty, either: with
> >>> > occur-edit-mode it is solved by making occur-edit-mode be derived from
> >>> > occur-mode.  Couldn't you do the same with your mode?
> >>> 
> >>> No because occur-mode makes occur-revert-arguments permanent-local so
> >>> `g' survives the major-mode changes.
> >>> 
> >>> For revert-buffer alone, compilation-arguments needs to be marked
> >>> permanent-local.  As it is a part of compile.el, I am not sure if
> >>> marking it as such is safe.  This is why I think having a minor-mode is
> >>> better.
> >>
> >> It sounds like a minor issue which shouldn't have such grave
> >> consequences.  Why do you think making compilation-arguments
> >> permanent-local would be a problem?  We could ask people who
> >> frequently contribute to compile.e land grep.el if they see any
> >> problem with doing that.
> >
> > It feels like a potential far-fetching change.  But in any case, I will
> > give a shot at recreating the required markers, etc. for
> > occur-after-change-function to work.  Will send a patch once I have a
> > working prototype.
> 
> OK, please find attached patch.  I am still not using a major-mode
> because of the change-major-mode-hook in compilation-setup:
> 
>     (add-hook 'change-major-mode-hook #'compilation--remove-properties nil t)
> 
> that strips off all the text-properties.  And there seem to be too many
> important local variables that is set up by both grep.el and compile.el
> so I think it is easier to use a minor-mode here.

Any reason not to modify compilation--remove-properties not to remove
all the text properties in this particular case?  AFAIU,
change-major-mode-hook is run by kill-all-local-variables, which the
major mode should call, so just before that you could remove
compilation--remove-properties from change-major-mode-hook, and that
would disable this face removal, no?

(I must admit that unconditionally removing the faces is not very
friendly on the part of compilation-mode.  Stefan, are there better
ways of doing that, perhaps?)





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-07 17:23 ` Jim Porter
  2024-05-08  3:12   ` Visuwesh
@ 2024-05-18 13:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 34+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-18 13:23 UTC (permalink / raw)
  To: Jim Porter; +Cc: 70820, Visuwesh

> How does this compare to the existing wgrep package? That doesn't have
> copyright assignment to the FSF, but otherwise, I think it's the bar against
> which any similar package should be measured. I use it pretty frequently
> (and have even written a plugin for it for my own grep-like major mode), and
> it's pretty much exactly the way I'd want this sort of thing to
> work. Something with feature-parity that lives in the Emacs tree would be
> great though.

FWIW, I don't see "live in Emacs tree" as particularly desirable for
such a user-oriented package.  It's already in NonGNU ELPA, so trivially
installable.

Note: I have not looked at the code of `wgrep.el`, so maybe there are
good reasons to reinvent this wheel, but "live in the Emacs tree" isn't
a good reason.


        Stefan






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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-08 13:49         ` Eli Zaretskii
  2024-05-09 10:32           ` Visuwesh
@ 2024-05-18 13:34           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 34+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-18 13:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, Visuwesh

>> > I don't think I understand this difficulty, either: with
>> > occur-edit-mode it is solved by making occur-edit-mode be derived from
>> > occur-mode.  Couldn't you do the same with your mode?
>> 
>> No because occur-mode makes occur-revert-arguments permanent-local so
>> `g' survives the major-mode changes.
>> 
>> For revert-buffer alone, compilation-arguments needs to be marked
>> permanent-local.  As it is a part of compile.el, I am not sure if
>> marking it as such is safe.  This is why I think having a minor-mode is
>> better.
>
> It sounds like a minor issue which shouldn't have such grave
> consequences.  Why do you think making compilation-arguments
> permanent-local would be a problem?  We could ask people who
> frequently contribute to compile.e land grep.el if they see any
> problem with doing that.

AFAICT it would be harmless to make it permanent-local.  Indeed, this
var belongs with the contents of the buffer rather than with its mode
(in a sense it is akin to `buffer-file-name` in that it says where the
contents comes from), so it makes a lot of sense to mark it
permanent-local.

Side note: I believe things become simpler&cleaner if you separate the
major mode function from the command that switches to it (just like
`wdired-change-to-wdired-mode` is separate from `wdired-mode`).

E.g. that lets you save&restore buffer-local vars like
`compilation-arguments`, and it let you remove or disable
`compilation--remove-properties` in a straightforward manner.


        Stefan






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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-18  9:28               ` Eli Zaretskii
@ 2024-05-18 13:35                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-18 15:44                   ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-18 13:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, Visuwesh

> (I must admit that unconditionally removing the faces is not very
> friendly on the part of compilation-mode.  Stefan, are there better
> ways of doing that, perhaps?)

These are properties which the major mode (unconditionally) added, so it
doesn't seem unfriendly to remove them unconditionally when switching to
an unrelated major mode.


        Stefan






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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-18 13:35                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-18 15:44                   ` Eli Zaretskii
  2024-05-18 16:27                     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-18 15:44 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 70820, visuweshm

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Visuwesh <visuweshm@gmail.com>,  70820@debbugs.gnu.org
> Date: Sat, 18 May 2024 09:35:58 -0400
> 
> > (I must admit that unconditionally removing the faces is not very
> > friendly on the part of compilation-mode.  Stefan, are there better
> > ways of doing that, perhaps?)
> 
> These are properties which the major mode (unconditionally) added, so it
> doesn't seem unfriendly to remove them unconditionally when switching to
> an unrelated major mode.

Yes, but how does compilation-mode know the other major-mode is
"unrelated"?  It could be very related, as this discussion
demonstrates.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-18 15:44                   ` Eli Zaretskii
@ 2024-05-18 16:27                     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-18 16:28                       ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-18 16:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, visuweshm

>> These are properties which the major mode (unconditionally) added, so it
>> doesn't seem unfriendly to remove them unconditionally when switching to
>> an unrelated major mode.
> Yes, but how does compilation-mode know the other major-mode is
> "unrelated"?  It could be very related, as this discussion
> demonstrates.

If it's related it has access to internals, so it can so things like
disabling `compilation--remoave-properties`.


        Stefan






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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-18 16:27                     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-18 16:28                       ` Eli Zaretskii
  2024-05-20 10:10                         ` Visuwesh
  2024-07-28  8:33                         ` Visuwesh
  0 siblings, 2 replies; 34+ messages in thread
From: Eli Zaretskii @ 2024-05-18 16:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 70820, visuweshm

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: visuweshm@gmail.com,  70820@debbugs.gnu.org
> Date: Sat, 18 May 2024 12:27:06 -0400
> 
> >> These are properties which the major mode (unconditionally) added, so it
> >> doesn't seem unfriendly to remove them unconditionally when switching to
> >> an unrelated major mode.
> > Yes, but how does compilation-mode know the other major-mode is
> > "unrelated"?  It could be very related, as this discussion
> > demonstrates.
> 
> If it's related it has access to internals, so it can so things like
> disabling `compilation--remoave-properties`.

OK, so my suggestion to Visuwesh is the best way of dealing with this.
Fine by me.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-18 16:28                       ` Eli Zaretskii
@ 2024-05-20 10:10                         ` Visuwesh
  2024-07-28  8:33                         ` Visuwesh
  1 sibling, 0 replies; 34+ messages in thread
From: Visuwesh @ 2024-05-20 10:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, Stefan Monnier

[சனி மே 18, 2024] Eli Zaretskii wrote:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: visuweshm@gmail.com,  70820@debbugs.gnu.org
>> Date: Sat, 18 May 2024 12:27:06 -0400
>> 
>> >> These are properties which the major mode (unconditionally) added, so it
>> >> doesn't seem unfriendly to remove them unconditionally when switching to
>> >> an unrelated major mode.
>> > Yes, but how does compilation-mode know the other major-mode is
>> > "unrelated"?  It could be very related, as this discussion
>> > demonstrates.
>> 
>> If it's related it has access to internals, so it can so things like
>> disabling `compilation--remoave-properties`.
>
> OK, so my suggestion to Visuwesh is the best way of dealing with this.
> Fine by me.

Thank you Stefan & Eli, I will take a closer look at your suggestions
when time permits.  I have limited internet access outside my work hours
so I cannot reply to your messages in full.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-05-18 16:28                       ` Eli Zaretskii
  2024-05-20 10:10                         ` Visuwesh
@ 2024-07-28  8:33                         ` Visuwesh
  2024-08-14  0:30                           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 34+ messages in thread
From: Visuwesh @ 2024-07-28  8:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, Stefan Monnier

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

[சனி மே 18, 2024] Eli Zaretskii wrote:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: visuweshm@gmail.com,  70820@debbugs.gnu.org
>> Date: Sat, 18 May 2024 12:27:06 -0400
>> 
>> >> These are properties which the major mode (unconditionally) added, so it
>> >> doesn't seem unfriendly to remove them unconditionally when switching to
>> >> an unrelated major mode.
>> > Yes, but how does compilation-mode know the other major-mode is
>> > "unrelated"?  It could be very related, as this discussion
>> > demonstrates.
>> 
>> If it's related it has access to internals, so it can so things like
>> disabling `compilation--remoave-properties`.
>
> OK, so my suggestion to Visuwesh is the best way of dealing with this.
> Fine by me.

Sorry for the long delay.  I finally got the time to work on this
properly.  The attached patch uses a major-mode, akin to wdired-mode
like Stefan suggested in another message.  This side-steps previous
issues with buffer-local variables and compilation--remove-properties.
If this approach is okay, I will work on updating the documentation,
etc.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grep-edit-v2.diff --]
[-- Type: text/x-diff, Size: 10602 bytes --]

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index b18eb81fee1..867ebc92d75 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2829,6 +2829,53 @@ compilation-find-buffer
       (current-buffer)
     (next-error-find-buffer avoid-current 'compilation-buffer-internal-p)))

+(defun compilation--update-markers (loc marker screen-columns first-column)
+  "Update markers in LOC, and set MARKER to location pointed by LOC.
+SCREEN-COLUMNS and FIRST-COLUMN are the value of
+`compilation-error-screen-columns' and `compilation-first-column' to use
+if they are not set buffer-locally in the target buffer."
+  (with-current-buffer
+      (if (bufferp (caar (compilation--loc->file-struct loc)))
+          (caar (compilation--loc->file-struct loc))
+        (apply #'compilation-find-file
+               marker
+               (caar (compilation--loc->file-struct loc))
+               (cadr (car (compilation--loc->file-struct loc)))
+               (compilation--file-struct->formats
+                (compilation--loc->file-struct loc))))
+    (let ((screen-columns
+           ;; Obey the compilation-error-screen-columns of the target
+           ;; buffer if its major mode set it buffer-locally.
+           (if (local-variable-p 'compilation-error-screen-columns)
+               compilation-error-screen-columns screen-columns))
+          (compilation-first-column
+           (if (local-variable-p 'compilation-first-column)
+               compilation-first-column first-column))
+          (last 1))
+      (save-restriction
+        (widen)
+        (goto-char (point-min))
+        ;; Treat file's found lines in forward order, 1 by 1.
+        (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
+          (when (car line)     ; else this is a filename without a line#
+            (compilation-beginning-of-line (- (car line) last -1))
+            (setq last (car line)))
+          ;; Treat line's found columns and store/update a marker for each.
+          (dolist (col (cdr line))
+            (if (compilation--loc->col col)
+                (if (eq (compilation--loc->col col) -1)
+                    ;; Special case for range end.
+                    (end-of-line)
+                  (compilation-move-to-column (compilation--loc->col col)
+                                              screen-columns))
+              (beginning-of-line)
+              (skip-chars-forward " \t"))
+            (if (compilation--loc->marker col)
+                (set-marker (compilation--loc->marker col) (point))
+              (setf (compilation--loc->marker col) (point-marker)))
+            ;; (setf (compilation--loc->timestamp col) timestamp)
+            ))))))
+
 ;;;###autoload
 (defun compilation-next-error-function (n &optional reset)
   "Advance to the next error message and visit the file where the error was.
@@ -2838,7 +2885,6 @@ compilation-next-error-function
     (setq compilation-current-error nil))
   (let* ((screen-columns compilation-error-screen-columns)
 	 (first-column compilation-first-column)
-	 (last 1)
 	 (msg (compilation-next-error (or n 1) nil
 				      (or compilation-current-error
 					  compilation-messages-start
@@ -2850,9 +2896,9 @@ compilation-next-error-function
       (user-error "No next error"))
     (setq compilation-current-error (point-marker)
 	  overlay-arrow-position
-	    (if (bolp)
-		compilation-current-error
-	      (copy-marker (line-beginning-position))))
+	  (if (bolp)
+	      compilation-current-error
+	    (copy-marker (line-beginning-position))))
     ;; If loc contains no marker, no error in that file has been visited.
     ;; If the marker is invalid the buffer has been killed.
     ;; So, recalculate all markers for that file.
@@ -2869,46 +2915,7 @@ compilation-next-error-function
                  ;;     (equal (compilation--loc->timestamp loc)
                  ;;            (setq timestamp compilation-buffer-modtime)))
                  )
-      (with-current-buffer
-          (if (bufferp (caar (compilation--loc->file-struct loc)))
-              (caar (compilation--loc->file-struct loc))
-            (apply #'compilation-find-file
-                   marker
-                   (caar (compilation--loc->file-struct loc))
-                   (cadr (car (compilation--loc->file-struct loc)))
-                   (compilation--file-struct->formats
-                    (compilation--loc->file-struct loc))))
-        (let ((screen-columns
-               ;; Obey the compilation-error-screen-columns of the target
-               ;; buffer if its major mode set it buffer-locally.
-               (if (local-variable-p 'compilation-error-screen-columns)
-                   compilation-error-screen-columns screen-columns))
-              (compilation-first-column
-               (if (local-variable-p 'compilation-first-column)
-                   compilation-first-column first-column)))
-          (save-restriction
-            (widen)
-            (goto-char (point-min))
-            ;; Treat file's found lines in forward order, 1 by 1.
-            (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
-              (when (car line)		; else this is a filename without a line#
-                (compilation-beginning-of-line (- (car line) last -1))
-                (setq last (car line)))
-              ;; Treat line's found columns and store/update a marker for each.
-              (dolist (col (cdr line))
-                (if (compilation--loc->col col)
-                    (if (eq (compilation--loc->col col) -1)
-                        ;; Special case for range end.
-                        (end-of-line)
-                      (compilation-move-to-column (compilation--loc->col col)
-                                                  screen-columns))
-                  (beginning-of-line)
-                  (skip-chars-forward " \t"))
-                (if (compilation--loc->marker col)
-                    (set-marker (compilation--loc->marker col) (point))
-                  (setf (compilation--loc->marker col) (point-marker)))
-                ;; (setf (compilation--loc->timestamp col) timestamp)
-                ))))))
+      (compilation--update-markers loc marker screen-columns first-column))
     (compilation-goto-locus marker (compilation--loc->marker loc)
                             (compilation--loc->marker end-loc))
     (setf (compilation--loc->visited loc) t)))
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 657349cbdff..605e763b444 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -295,6 +295,8 @@ grep-mode-map
     (define-key map "}" #'compilation-next-file)
     (define-key map "\t" #'compilation-next-error)
     (define-key map [backtab] #'compilation-previous-error)
+
+    (define-key map "e" #'grep-change-to-grep-edit-mode)
     map)
   "Keymap for grep buffers.
 `compilation-minor-mode-map' is a cdr of this.")
@@ -1029,6 +1031,90 @@ grep
 		       command-args)
 		     #'grep-mode))

+(defun grep-edit--prepare-buffer ()
+  "Mark relevant regions read-only, and add relevant occur text-properties."
+  (save-excursion
+    (goto-char (point-min))
+    (let ((inhibit-read-only t)
+          (dummy (make-marker))
+          match)
+      (while (setq match (text-property-search-forward 'compilation-annotation))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t)))
+      (goto-char (point-min))
+      (while (setq match (text-property-search-forward 'compilation-message))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t occur-prefix t))
+        (let ((loc (compilation--message->loc (prop-match-value match)))
+              m)
+          ;; Update the markers if necessary.
+          (unless (and (compilation--loc->marker loc)
+                       (marker-buffer (compilation--loc->marker loc)))
+            (compilation--update-markers loc dummy compilation-error-screen-columns compilation-first-column))
+          (setq m (compilation--loc->marker loc))
+          (add-text-properties (prop-match-beginning match)
+                               (or (next-single-property-change
+                                    (prop-match-end match)
+                                    'compilation-message)
+                                   (1+ (pos-eol)))
+                               `(occur-target ((,m . ,m)))))))))
+
+(defvar grep-edit-mode-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map text-mode-map)
+    (define-key map (kbd "C-c C-c") #'grep-edit-save-changes)
+    map)
+  "Keymap for `grep-edit-mode'.")
+
+(defvar grep-edit-mode-hook nil
+  "Hooks run when changing to Grep-Edit mode.")
+
+(defun grep-edit-mode ()
+  "Major mode for editing *grep* buffers.
+In this mode, changes to the *grep* buffer are applied to the
+originating files.
+\\<grep-edit-mode-map>
+Type \\[grep-edit-save-changes] to exit Grep-Edit mode, return to Grep
+mode.
+
+The only editable texts in a Grep-Edit buffer are the match results."
+  (interactive)
+  (error "This mode can be enabled only by `grep-change-to-grep-edit-mode'"))
+(put 'grep-edit-mode 'mode-class 'special)
+
+(defun grep-change-to-grep-edit-mode ()
+  "Switch to `grep-edit-mode' to edit *grep* buffer."
+  (interactive)
+  (unless (derived-mode-p 'grep-mode)
+    (error "Not a Grep buffer"))
+  (when (get-buffer-process (current-buffer))
+    (error "Cannot switch when grep is running"))
+  (use-local-map grep-edit-mode-map)
+  (grep-edit--prepare-buffer)
+  (setq buffer-read-only nil)
+  (setq major-mode 'grep-edit-mode)
+  (setq mode-name "Grep-Edit")
+  (buffer-enable-undo)
+  (set-buffer-modified-p nil)
+  (setq buffer-undo-list nil)
+  (add-hook 'after-change-functions #'occur-after-change-function nil t)
+  (run-mode-hooks 'grep-edit-mode-hook)
+  (message "Editing: \\[grep-edit-save-changes] to return to Grep mode"))
+
+(defun grep-edit-save-changes ()
+  "Switch back to Grep mode."
+  (interactive)
+  (unless (derived-mode-p 'grep-edit-mode)
+    (error "Not a Grep-Edit buffer"))
+  (remove-hook 'after-change-functions #'occur-after-change-function t)
+  (use-local-map grep-mode-map)
+  (setq buffer-read-only t)
+  (setq major-mode 'grep-mode)
+  (setq mode-name "Grep")
+  (force-mode-line-update)
+  (buffer-disable-undo)
+  (setq buffer-undo-list t)
+  (message "Switching to Grep mode"))

 ;;;###autoload
 (defun grep-find (command-args)

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

* bug#70820: [PATCH] Editable grep buffers
  2024-07-28  8:33                         ` Visuwesh
@ 2024-08-14  0:30                           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-14  2:43                             ` Visuwesh
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-14  0:30 UTC (permalink / raw)
  To: Visuwesh; +Cc: Eli Zaretskii, 70820

> +(defun compilation--update-markers (loc marker screen-columns first-column)
> +  "Update markers in LOC, and set MARKER to location pointed by LOC.
> +SCREEN-COLUMNS and FIRST-COLUMN are the value of
> +`compilation-error-screen-columns' and `compilation-first-column' to use
> +if they are not set buffer-locally in the target buffer."
> +  (with-current-buffer
> +      (if (bufferp (caar (compilation--loc->file-struct loc)))
> +          (caar (compilation--loc->file-struct loc))
> +        (apply #'compilation-find-file
> +               marker
> +               (caar (compilation--loc->file-struct loc))
> +               (cadr (car (compilation--loc->file-struct loc)))
> +               (compilation--file-struct->formats
> +                (compilation--loc->file-struct loc))))
> +    (let ((screen-columns
> +           ;; Obey the compilation-error-screen-columns of the target
> +           ;; buffer if its major mode set it buffer-locally.
> +           (if (local-variable-p 'compilation-error-screen-columns)
> +               compilation-error-screen-columns screen-columns))
> +          (compilation-first-column
> +           (if (local-variable-p 'compilation-first-column)
> +               compilation-first-column first-column))
> +          (last 1))
> +      (save-restriction
> +        (widen)
> +        (goto-char (point-min))
> +        ;; Treat file's found lines in forward order, 1 by 1.
> +        (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
> +          (when (car line)     ; else this is a filename without a line#
> +            (compilation-beginning-of-line (- (car line) last -1))
> +            (setq last (car line)))
> +          ;; Treat line's found columns and store/update a marker for each.
> +          (dolist (col (cdr line))
> +            (if (compilation--loc->col col)
> +                (if (eq (compilation--loc->col col) -1)
> +                    ;; Special case for range end.
> +                    (end-of-line)
> +                  (compilation-move-to-column (compilation--loc->col col)
> +                                              screen-columns))
> +              (beginning-of-line)
> +              (skip-chars-forward " \t"))
> +            (if (compilation--loc->marker col)
> +                (set-marker (compilation--loc->marker col) (point))
> +              (setf (compilation--loc->marker col) (point-marker)))
> +            ;; (setf (compilation--loc->timestamp col) timestamp)
> +            ))))))

Are there any changes in this code, or is it "verbatim" the code
extracted from `compilation-next-error-function`?

> +(defvar grep-edit-mode-hook nil
> +  "Hooks run when changing to Grep-Edit mode.")

It's just "Hook" because `grep-edit-mode-hook` is a hook.  Hooks contain
functions, so when you run a hook, the corresponding functions are called.

> +(defun grep-edit-mode ()
> +  "Major mode for editing *grep* buffers.
> +In this mode, changes to the *grep* buffer are applied to the
> +originating files.
> +\\<grep-edit-mode-map>
> +Type \\[grep-edit-save-changes] to exit Grep-Edit mode, return to Grep
> +mode.
> +
> +The only editable texts in a Grep-Edit buffer are the match results."
> +  (interactive)
> +  (error "This mode can be enabled only by `grep-change-to-grep-edit-mode'"))
> +(put 'grep-edit-mode 'mode-class 'special)
> +
> +(defun grep-change-to-grep-edit-mode ()
> +  "Switch to `grep-edit-mode' to edit *grep* buffer."
> +  (interactive)
> +  (unless (derived-mode-p 'grep-mode)
> +    (error "Not a Grep buffer"))
> +  (when (get-buffer-process (current-buffer))
> +    (error "Cannot switch when grep is running"))
> +  (use-local-map grep-edit-mode-map)
> +  (grep-edit--prepare-buffer)
> +  (setq buffer-read-only nil)
> +  (setq major-mode 'grep-edit-mode)
> +  (setq mode-name "Grep-Edit")
> +  (buffer-enable-undo)
> +  (set-buffer-modified-p nil)
> +  (setq buffer-undo-list nil)
> +  (add-hook 'after-change-functions #'occur-after-change-function nil t)
> +  (run-mode-hooks 'grep-edit-mode-hook)
> +  (message "Editing: \\[grep-edit-save-changes] to return to Grep mode"))

I'm tempted to say you should use `major-mode-suspend/resume` (which
would avoid the duplication of parts of `grep-mode` in
`grep-edit-save-changes`), but I guess this might re-introduce the
problem with buffer-local variables.


        Stefan






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

* bug#70820: [PATCH] Editable grep buffers
  2024-08-14  0:30                           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-08-14  2:43                             ` Visuwesh
  2024-08-14 11:37                               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
                                                 ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Visuwesh @ 2024-08-14  2:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, 70820

[செவ்வாய் ஆகஸ்ட் 13, 2024] Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:

>> +(defun compilation--update-markers (loc marker screen-columns first-column)
>> +  "Update markers in LOC, and set MARKER to location pointed by LOC.
>> +SCREEN-COLUMNS and FIRST-COLUMN are the value of
>> +`compilation-error-screen-columns' and `compilation-first-column' to use
>> +if they are not set buffer-locally in the target buffer."
>> +  (with-current-buffer
>> +      (if (bufferp (caar (compilation--loc->file-struct loc)))
>> +          (caar (compilation--loc->file-struct loc))
>> +        (apply #'compilation-find-file
>> +               marker
>> +               (caar (compilation--loc->file-struct loc))
>> +               (cadr (car (compilation--loc->file-struct loc)))
>> +               (compilation--file-struct->formats
>> +                (compilation--loc->file-struct loc))))
>> +    (let ((screen-columns
>> +           ;; Obey the compilation-error-screen-columns of the target
>> +           ;; buffer if its major mode set it buffer-locally.
>> +           (if (local-variable-p 'compilation-error-screen-columns)
>> +               compilation-error-screen-columns screen-columns))
>> +          (compilation-first-column
>> +           (if (local-variable-p 'compilation-first-column)
>> +               compilation-first-column first-column))
>> +          (last 1))
>> +      (save-restriction
>> +        (widen)
>> +        (goto-char (point-min))
>> +        ;; Treat file's found lines in forward order, 1 by 1.
>> +        (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
>> +          (when (car line)     ; else this is a filename without a line#
>> +            (compilation-beginning-of-line (- (car line) last -1))
>> +            (setq last (car line)))
>> +          ;; Treat line's found columns and store/update a marker for each.
>> +          (dolist (col (cdr line))
>> +            (if (compilation--loc->col col)
>> +                (if (eq (compilation--loc->col col) -1)
>> +                    ;; Special case for range end.
>> +                    (end-of-line)
>> +                  (compilation-move-to-column (compilation--loc->col col)
>> +                                              screen-columns))
>> +              (beginning-of-line)
>> +              (skip-chars-forward " \t"))
>> +            (if (compilation--loc->marker col)
>> +                (set-marker (compilation--loc->marker col) (point))
>> +              (setf (compilation--loc->marker col) (point-marker)))
>> +            ;; (setf (compilation--loc->timestamp col) timestamp)
>> +            ))))))
>
> Are there any changes in this code, or is it "verbatim" the code
> extracted from `compilation-next-error-function`?

It is extracted verbatim from compilation-next-error-function.

>> +(defvar grep-edit-mode-hook nil
>> +  "Hooks run when changing to Grep-Edit mode.")
>
> It's just "Hook" because `grep-edit-mode-hook` is a hook.  Hooks contain
> functions, so when you run a hook, the corresponding functions are called.

Ah, I was under the impression that the functions were also referred to
as hooks.  I will correct them in a future patch.

>> +(defun grep-edit-mode ()
>> +  "Major mode for editing *grep* buffers.
>> +In this mode, changes to the *grep* buffer are applied to the
>> +originating files.
>> +\\<grep-edit-mode-map>
>> +Type \\[grep-edit-save-changes] to exit Grep-Edit mode, return to Grep
>> +mode.
>> +
>> +The only editable texts in a Grep-Edit buffer are the match results."
>> +  (interactive)
>> +  (error "This mode can be enabled only by `grep-change-to-grep-edit-mode'"))
>> +(put 'grep-edit-mode 'mode-class 'special)
>> +
>> +(defun grep-change-to-grep-edit-mode ()
>> +  "Switch to `grep-edit-mode' to edit *grep* buffer."
>> +  (interactive)
>> +  (unless (derived-mode-p 'grep-mode)
>> +    (error "Not a Grep buffer"))
>> +  (when (get-buffer-process (current-buffer))
>> +    (error "Cannot switch when grep is running"))
>> +  (use-local-map grep-edit-mode-map)
>> +  (grep-edit--prepare-buffer)
>> +  (setq buffer-read-only nil)
>> +  (setq major-mode 'grep-edit-mode)
>> +  (setq mode-name "Grep-Edit")
>> +  (buffer-enable-undo)
>> +  (set-buffer-modified-p nil)
>> +  (setq buffer-undo-list nil)
>> +  (add-hook 'after-change-functions #'occur-after-change-function nil t)
>> +  (run-mode-hooks 'grep-edit-mode-hook)
>> +  (message "Editing: \\[grep-edit-save-changes] to return to Grep mode"))
>
> I'm tempted to say you should use `major-mode-suspend/resume` (which
> would avoid the duplication of parts of `grep-mode` in
> `grep-edit-save-changes`), but I guess this might re-introduce the
> problem with buffer-local variables.

Yes, unfortunately it will re-introduce the problem with buffer-local
variables.  

If everyone is okay with the current patch, I can get to writing the
NEWS entry and updating the manual.  Thanks.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-08-14  2:43                             ` Visuwesh
@ 2024-08-14 11:37                               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-31  7:54                               ` Eli Zaretskii
  2024-09-09 14:39                               ` Visuwesh
  2 siblings, 0 replies; 34+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-14 11:37 UTC (permalink / raw)
  To: Visuwesh; +Cc: Eli Zaretskii, 70820

>> Are there any changes in this code, or is it "verbatim" the code
>> extracted from `compilation-next-error-function`?
> It is extracted verbatim from compilation-next-error-function.

Great, thanks.

>>> +(defvar grep-edit-mode-hook nil
>>> +  "Hooks run when changing to Grep-Edit mode.")
>> It's just "Hook" because `grep-edit-mode-hook` is a hook.  Hooks contain
>> functions, so when you run a hook, the corresponding functions are called.
> Ah, I was under the impression that the functions were also referred to
> as hooks.

You're far from the only one: it's a common confusion, and it's frequent
to see docstrings which encourage that confusion.

> If everyone is okay with the current patch, I can get to writing the
> NEWS entry and updating the manual.  Thanks.

+1


        Stefan






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

* bug#70820: [PATCH] Editable grep buffers
  2024-08-14  2:43                             ` Visuwesh
  2024-08-14 11:37                               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-08-31  7:54                               ` Eli Zaretskii
  2024-08-31  8:03                                 ` Visuwesh
  2024-09-09 14:39                               ` Visuwesh
  2 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2024-08-31  7:54 UTC (permalink / raw)
  To: Visuwesh; +Cc: 70820, monnier

Ping! Can we make progress with this issue?  I think there were no
objections to your proposal.

> From: Visuwesh <visuweshm@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  70820@debbugs.gnu.org
> Date: Wed, 14 Aug 2024 08:13:42 +0530
> 
> [செவ்வாய் ஆகஸ்ட் 13, 2024] Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:
> 
> >> +(defun compilation--update-markers (loc marker screen-columns first-column)
> >> +  "Update markers in LOC, and set MARKER to location pointed by LOC.
> >> +SCREEN-COLUMNS and FIRST-COLUMN are the value of
> >> +`compilation-error-screen-columns' and `compilation-first-column' to use
> >> +if they are not set buffer-locally in the target buffer."
> >> +  (with-current-buffer
> >> +      (if (bufferp (caar (compilation--loc->file-struct loc)))
> >> +          (caar (compilation--loc->file-struct loc))
> >> +        (apply #'compilation-find-file
> >> +               marker
> >> +               (caar (compilation--loc->file-struct loc))
> >> +               (cadr (car (compilation--loc->file-struct loc)))
> >> +               (compilation--file-struct->formats
> >> +                (compilation--loc->file-struct loc))))
> >> +    (let ((screen-columns
> >> +           ;; Obey the compilation-error-screen-columns of the target
> >> +           ;; buffer if its major mode set it buffer-locally.
> >> +           (if (local-variable-p 'compilation-error-screen-columns)
> >> +               compilation-error-screen-columns screen-columns))
> >> +          (compilation-first-column
> >> +           (if (local-variable-p 'compilation-first-column)
> >> +               compilation-first-column first-column))
> >> +          (last 1))
> >> +      (save-restriction
> >> +        (widen)
> >> +        (goto-char (point-min))
> >> +        ;; Treat file's found lines in forward order, 1 by 1.
> >> +        (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
> >> +          (when (car line)     ; else this is a filename without a line#
> >> +            (compilation-beginning-of-line (- (car line) last -1))
> >> +            (setq last (car line)))
> >> +          ;; Treat line's found columns and store/update a marker for each.
> >> +          (dolist (col (cdr line))
> >> +            (if (compilation--loc->col col)
> >> +                (if (eq (compilation--loc->col col) -1)
> >> +                    ;; Special case for range end.
> >> +                    (end-of-line)
> >> +                  (compilation-move-to-column (compilation--loc->col col)
> >> +                                              screen-columns))
> >> +              (beginning-of-line)
> >> +              (skip-chars-forward " \t"))
> >> +            (if (compilation--loc->marker col)
> >> +                (set-marker (compilation--loc->marker col) (point))
> >> +              (setf (compilation--loc->marker col) (point-marker)))
> >> +            ;; (setf (compilation--loc->timestamp col) timestamp)
> >> +            ))))))
> >
> > Are there any changes in this code, or is it "verbatim" the code
> > extracted from `compilation-next-error-function`?
> 
> It is extracted verbatim from compilation-next-error-function.
> 
> >> +(defvar grep-edit-mode-hook nil
> >> +  "Hooks run when changing to Grep-Edit mode.")
> >
> > It's just "Hook" because `grep-edit-mode-hook` is a hook.  Hooks contain
> > functions, so when you run a hook, the corresponding functions are called.
> 
> Ah, I was under the impression that the functions were also referred to
> as hooks.  I will correct them in a future patch.
> 
> >> +(defun grep-edit-mode ()
> >> +  "Major mode for editing *grep* buffers.
> >> +In this mode, changes to the *grep* buffer are applied to the
> >> +originating files.
> >> +\\<grep-edit-mode-map>
> >> +Type \\[grep-edit-save-changes] to exit Grep-Edit mode, return to Grep
> >> +mode.
> >> +
> >> +The only editable texts in a Grep-Edit buffer are the match results."
> >> +  (interactive)
> >> +  (error "This mode can be enabled only by `grep-change-to-grep-edit-mode'"))
> >> +(put 'grep-edit-mode 'mode-class 'special)
> >> +
> >> +(defun grep-change-to-grep-edit-mode ()
> >> +  "Switch to `grep-edit-mode' to edit *grep* buffer."
> >> +  (interactive)
> >> +  (unless (derived-mode-p 'grep-mode)
> >> +    (error "Not a Grep buffer"))
> >> +  (when (get-buffer-process (current-buffer))
> >> +    (error "Cannot switch when grep is running"))
> >> +  (use-local-map grep-edit-mode-map)
> >> +  (grep-edit--prepare-buffer)
> >> +  (setq buffer-read-only nil)
> >> +  (setq major-mode 'grep-edit-mode)
> >> +  (setq mode-name "Grep-Edit")
> >> +  (buffer-enable-undo)
> >> +  (set-buffer-modified-p nil)
> >> +  (setq buffer-undo-list nil)
> >> +  (add-hook 'after-change-functions #'occur-after-change-function nil t)
> >> +  (run-mode-hooks 'grep-edit-mode-hook)
> >> +  (message "Editing: \\[grep-edit-save-changes] to return to Grep mode"))
> >
> > I'm tempted to say you should use `major-mode-suspend/resume` (which
> > would avoid the duplication of parts of `grep-mode` in
> > `grep-edit-save-changes`), but I guess this might re-introduce the
> > problem with buffer-local variables.
> 
> Yes, unfortunately it will re-introduce the problem with buffer-local
> variables.  
> 
> If everyone is okay with the current patch, I can get to writing the
> NEWS entry and updating the manual.  Thanks.
> 





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

* bug#70820: [PATCH] Editable grep buffers
  2024-08-31  7:54                               ` Eli Zaretskii
@ 2024-08-31  8:03                                 ` Visuwesh
  0 siblings, 0 replies; 34+ messages in thread
From: Visuwesh @ 2024-08-31  8:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70820, monnier

[சனி ஆகஸ்ட் 31, 2024] Eli Zaretskii wrote:

> Ping! Can we make progress with this issue?  I think there were no
> objections to your proposal.

I will send a proper patch in the coming week then.





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

* bug#70820: [PATCH] Editable grep buffers
  2024-08-14  2:43                             ` Visuwesh
  2024-08-14 11:37                               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-31  7:54                               ` Eli Zaretskii
@ 2024-09-09 14:39                               ` Visuwesh
  2024-09-14  9:43                                 ` Eli Zaretskii
  2 siblings, 1 reply; 34+ messages in thread
From: Visuwesh @ 2024-09-09 14:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, 70820

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

[புதன் ஆகஸ்ட் 14, 2024] Visuwesh wrote:

> [...]
> If everyone is okay with the current patch, I can get to writing the
> NEWS entry and updating the manual.  Thanks.

Sorry for the long delay.  I've now attached a patch with such
documentation changes.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-the-grep-buffer-editable.patch --]
[-- Type: text/x-diff, Size: 13252 bytes --]

From 52e4d82dd3f28065e5e3ea390c4629783bcf139d Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Mon, 9 Sep 2024 20:08:04 +0530
Subject: [PATCH] Make the *grep* buffer editable

* lisp/progmodes/compile.el (compilation--update-markers):
Factor out function...
(compilation-next-error-function): from here.  Adjust to use
above.
* lisp/progmodes/grep.el (grep-edit--prepare-buffer)
(grep-edit-mode-map, grep-edit-mode-hook, grep-edit-mode)
(grep-change-to-grep-edit-mode, grep-edit-save-changes): Add new
grep-edit-mode to make the grep results editable like in
'occur-edit-mode' by using the 'occur' framework.
(grep-mode-map): Bind 'e' to the new command
'grep-change-to-grep-edit-mode'.
* doc/emacs/building.texi (Grep Searching): Update Info manual
to include the above command.
* etc/NEWS: Announce the change.  (bug#70820)
---
 doc/emacs/building.texi   | 10 +++++
 etc/NEWS                  |  9 ++++
 lisp/progmodes/compile.el | 95 +++++++++++++++++++++------------------
 lisp/progmodes/grep.el    | 86 +++++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 44 deletions(-)

diff --git a/doc/emacs/building.texi b/doc/emacs/building.texi
index bb03d8cf325..4b2f1ed0649 100644
--- a/doc/emacs/building.texi
+++ b/doc/emacs/building.texi
@@ -528,6 +528,16 @@ Grep Searching
 shell commands, customize the option @code{grep-find-abbreviate} to a
 @code{nil} value.
 
+@findex grep-change-to-grep-edit-mode
+@cindex Grep Edit mode
+@cindex mode, Grep Edit
+  Typing @kbd{e} in the @file{*grep*} buffer makes the buffer writiable
+and enters the Grep Edit mode.  Similar to Occur Edit mode (@pxref{Other
+Repeating Search}), you can edit the matching lines reported by
+@code{grep} and have those changes reflected in the buffer visiting the
+originating file.  Type @kbd{C-c C-c} to leave the Grep Edit mode and
+return to the Grep mode.
+
 @node Flymake
 @section Finding Syntax Errors On The Fly
 @cindex checking syntax
diff --git a/etc/NEWS b/etc/NEWS
index c6f8b0062e4..24b43108ecb 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -290,6 +290,15 @@ the built-in Web server.  Interactively, when invoked with a prefix
 argument, 'php-ts-mode-run-php-webserver' prompts for the config file as
 well as for other connection parameters.
 
+** Grep
+
++++
+*** Grep results can be edited to reflect changes in the originating file.
+Like Occur Edit mode, typing 'e' in the '*grep*' buffer will now make
+the 'grep' results editable.  The edits will be reflected in the buffer
+visiting the originating file.  Typing 'C-c C-c' will leave the Grep
+Edit mode.
+
 \f
 * New Modes and Packages in Emacs 31.1
 
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index d2e74aa44a6..a78ac1b6462 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2855,6 +2855,53 @@ compilation-find-buffer
       (current-buffer)
     (next-error-find-buffer avoid-current 'compilation-buffer-internal-p)))
 
+(defun compilation--update-markers (loc marker screen-columns first-column)
+  "Update markers in LOC, and set MARKER to location pointed by LOC.
+SCREEN-COLUMNS and FIRST-COLUMN are the value of
+`compilation-error-screen-columns' and `compilation-first-column' to use
+if they are not set buffer-locally in the target buffer."
+  (with-current-buffer
+      (if (bufferp (caar (compilation--loc->file-struct loc)))
+          (caar (compilation--loc->file-struct loc))
+        (apply #'compilation-find-file
+               marker
+               (caar (compilation--loc->file-struct loc))
+               (cadr (car (compilation--loc->file-struct loc)))
+               (compilation--file-struct->formats
+                (compilation--loc->file-struct loc))))
+    (let ((screen-columns
+           ;; Obey the compilation-error-screen-columns of the target
+           ;; buffer if its major mode set it buffer-locally.
+           (if (local-variable-p 'compilation-error-screen-columns)
+               compilation-error-screen-columns screen-columns))
+          (compilation-first-column
+           (if (local-variable-p 'compilation-first-column)
+               compilation-first-column first-column))
+          (last 1))
+      (save-restriction
+        (widen)
+        (goto-char (point-min))
+        ;; Treat file's found lines in forward order, 1 by 1.
+        (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
+          (when (car line)     ; else this is a filename without a line#
+            (compilation-beginning-of-line (- (car line) last -1))
+            (setq last (car line)))
+          ;; Treat line's found columns and store/update a marker for each.
+          (dolist (col (cdr line))
+            (if (compilation--loc->col col)
+                (if (eq (compilation--loc->col col) -1)
+                    ;; Special case for range end.
+                    (end-of-line)
+                  (compilation-move-to-column (compilation--loc->col col)
+                                              screen-columns))
+              (beginning-of-line)
+              (skip-chars-forward " \t"))
+            (if (compilation--loc->marker col)
+                (set-marker (compilation--loc->marker col) (point))
+              (setf (compilation--loc->marker col) (point-marker)))
+            ;; (setf (compilation--loc->timestamp col) timestamp)
+            ))))))
+
 ;;;###autoload
 (defun compilation-next-error-function (n &optional reset)
   "Advance to the next error message and visit the file where the error was.
@@ -2864,7 +2911,6 @@ compilation-next-error-function
     (setq compilation-current-error nil))
   (let* ((screen-columns compilation-error-screen-columns)
 	 (first-column compilation-first-column)
-	 (last 1)
 	 (msg (compilation-next-error (or n 1) nil
 				      (or compilation-current-error
 					  compilation-messages-start
@@ -2876,9 +2922,9 @@ compilation-next-error-function
       (user-error "No next error"))
     (setq compilation-current-error (point-marker)
 	  overlay-arrow-position
-	    (if (bolp)
-		compilation-current-error
-	      (copy-marker (line-beginning-position))))
+	  (if (bolp)
+	      compilation-current-error
+	    (copy-marker (line-beginning-position))))
     ;; If loc contains no marker, no error in that file has been visited.
     ;; If the marker is invalid the buffer has been killed.
     ;; So, recalculate all markers for that file.
@@ -2895,46 +2941,7 @@ compilation-next-error-function
                  ;;     (equal (compilation--loc->timestamp loc)
                  ;;            (setq timestamp compilation-buffer-modtime)))
                  )
-      (with-current-buffer
-          (if (bufferp (caar (compilation--loc->file-struct loc)))
-              (caar (compilation--loc->file-struct loc))
-            (apply #'compilation-find-file
-                   marker
-                   (caar (compilation--loc->file-struct loc))
-                   (cadr (car (compilation--loc->file-struct loc)))
-                   (compilation--file-struct->formats
-                    (compilation--loc->file-struct loc))))
-        (let ((screen-columns
-               ;; Obey the compilation-error-screen-columns of the target
-               ;; buffer if its major mode set it buffer-locally.
-               (if (local-variable-p 'compilation-error-screen-columns)
-                   compilation-error-screen-columns screen-columns))
-              (compilation-first-column
-               (if (local-variable-p 'compilation-first-column)
-                   compilation-first-column first-column)))
-          (save-restriction
-            (widen)
-            (goto-char (point-min))
-            ;; Treat file's found lines in forward order, 1 by 1.
-            (dolist (line (reverse (cddr (compilation--loc->file-struct loc))))
-              (when (car line)		; else this is a filename without a line#
-                (compilation-beginning-of-line (- (car line) last -1))
-                (setq last (car line)))
-              ;; Treat line's found columns and store/update a marker for each.
-              (dolist (col (cdr line))
-                (if (compilation--loc->col col)
-                    (if (eq (compilation--loc->col col) -1)
-                        ;; Special case for range end.
-                        (end-of-line)
-                      (compilation-move-to-column (compilation--loc->col col)
-                                                  screen-columns))
-                  (beginning-of-line)
-                  (skip-chars-forward " \t"))
-                (if (compilation--loc->marker col)
-                    (set-marker (compilation--loc->marker col) (point))
-                  (setf (compilation--loc->marker col) (point-marker)))
-                ;; (setf (compilation--loc->timestamp col) timestamp)
-                ))))))
+      (compilation--update-markers loc marker screen-columns first-column))
     (compilation-goto-locus marker (compilation--loc->marker loc)
                             (compilation--loc->marker end-loc))
     (setf (compilation--loc->visited loc) t)))
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index d2d0baa235c..54006560224 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -310,6 +310,8 @@ grep-mode-map
     (define-key map "}" #'compilation-next-file)
     (define-key map "\t" #'compilation-next-error)
     (define-key map [backtab] #'compilation-previous-error)
+
+    (define-key map "e" #'grep-change-to-grep-edit-mode)
     map)
   "Keymap for grep buffers.
 `compilation-minor-mode-map' is a cdr of this.")
@@ -1052,6 +1054,90 @@ grep
 		       command-args)
 		     #'grep-mode))
 
+(defun grep-edit--prepare-buffer ()
+  "Mark relevant regions read-only, and add relevant occur text-properties."
+  (save-excursion
+    (goto-char (point-min))
+    (let ((inhibit-read-only t)
+          (dummy (make-marker))
+          match)
+      (while (setq match (text-property-search-forward 'compilation-annotation))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t)))
+      (goto-char (point-min))
+      (while (setq match (text-property-search-forward 'compilation-message))
+        (add-text-properties (prop-match-beginning match) (prop-match-end match)
+                             '(read-only t occur-prefix t))
+        (let ((loc (compilation--message->loc (prop-match-value match)))
+              m)
+          ;; Update the markers if necessary.
+          (unless (and (compilation--loc->marker loc)
+                       (marker-buffer (compilation--loc->marker loc)))
+            (compilation--update-markers loc dummy compilation-error-screen-columns compilation-first-column))
+          (setq m (compilation--loc->marker loc))
+          (add-text-properties (prop-match-beginning match)
+                               (or (next-single-property-change
+                                    (prop-match-end match)
+                                    'compilation-message)
+                                   (1+ (pos-eol)))
+                               `(occur-target ((,m . ,m)))))))))
+
+(defvar grep-edit-mode-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map text-mode-map)
+    (define-key map (kbd "C-c C-c") #'grep-edit-save-changes)
+    map)
+  "Keymap for `grep-edit-mode'.")
+
+(defvar grep-edit-mode-hook nil
+  "Hooks run when changing to Grep-Edit mode.")
+
+(defun grep-edit-mode ()
+  "Major mode for editing *grep* buffers.
+In this mode, changes to the *grep* buffer are applied to the
+originating files.
+\\<grep-edit-mode-map>
+Type \\[grep-edit-save-changes] to exit Grep-Edit mode, return to Grep
+mode.
+
+The only editable texts in a Grep-Edit buffer are the match results."
+  (interactive)
+  (error "This mode can be enabled only by `grep-change-to-grep-edit-mode'"))
+(put 'grep-edit-mode 'mode-class 'special)
+
+(defun grep-change-to-grep-edit-mode ()
+  "Switch to `grep-edit-mode' to edit *grep* buffer."
+  (interactive)
+  (unless (derived-mode-p 'grep-mode)
+    (error "Not a Grep buffer"))
+  (when (get-buffer-process (current-buffer))
+    (error "Cannot switch when grep is running"))
+  (use-local-map grep-edit-mode-map)
+  (grep-edit--prepare-buffer)
+  (setq buffer-read-only nil)
+  (setq major-mode 'grep-edit-mode)
+  (setq mode-name "Grep-Edit")
+  (buffer-enable-undo)
+  (set-buffer-modified-p nil)
+  (setq buffer-undo-list nil)
+  (add-hook 'after-change-functions #'occur-after-change-function nil t)
+  (run-mode-hooks 'grep-edit-mode-hook)
+  (message "Editing: \\[grep-edit-save-changes] to return to Grep mode"))
+
+(defun grep-edit-save-changes ()
+  "Switch back to Grep mode."
+  (interactive)
+  (unless (derived-mode-p 'grep-edit-mode)
+    (error "Not a Grep-Edit buffer"))
+  (remove-hook 'after-change-functions #'occur-after-change-function t)
+  (use-local-map grep-mode-map)
+  (setq buffer-read-only t)
+  (setq major-mode 'grep-mode)
+  (setq mode-name "Grep")
+  (force-mode-line-update)
+  (buffer-disable-undo)
+  (setq buffer-undo-list t)
+  (message "Switching to Grep mode"))
 
 ;;;###autoload
 (defun grep-find (command-args)
-- 
2.45.2


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

* bug#70820: [PATCH] Editable grep buffers
  2024-09-09 14:39                               ` Visuwesh
@ 2024-09-14  9:43                                 ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2024-09-14  9:43 UTC (permalink / raw)
  To: Visuwesh; +Cc: 70820-done, monnier

> From: Visuwesh <visuweshm@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  70820@debbugs.gnu.org
> Date: Mon, 09 Sep 2024 20:09:22 +0530
> 
> [புதன் ஆகஸ்ட் 14, 2024] Visuwesh wrote:
> 
> > [...]
> > If everyone is okay with the current patch, I can get to writing the
> > NEWS entry and updating the manual.  Thanks.
> 
> Sorry for the long delay.  I've now attached a patch with such
> documentation changes.

Thanks, installed on the master branch, and closing the bug.





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

end of thread, other threads:[~2024-09-14  9:43 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-07 16:25 bug#70820: [PATCH] Editable grep buffers Visuwesh
2024-05-07 17:23 ` Jim Porter
2024-05-08  3:12   ` Visuwesh
2024-05-08  4:11     ` Jim Porter
2024-05-08  5:11       ` Visuwesh
2024-05-18 13:23   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-07 18:08 ` Eli Zaretskii
2024-05-08  3:22   ` Visuwesh
2024-05-08 11:58     ` Eli Zaretskii
2024-05-08 12:18       ` Visuwesh
2024-05-08 13:49         ` Eli Zaretskii
2024-05-09 10:32           ` Visuwesh
2024-05-12  4:45             ` Visuwesh
2024-05-18  9:28               ` Eli Zaretskii
2024-05-18 13:35                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-18 15:44                   ` Eli Zaretskii
2024-05-18 16:27                     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-18 16:28                       ` Eli Zaretskii
2024-05-20 10:10                         ` Visuwesh
2024-07-28  8:33                         ` Visuwesh
2024-08-14  0:30                           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-08-14  2:43                             ` Visuwesh
2024-08-14 11:37                               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-08-31  7:54                               ` Eli Zaretskii
2024-08-31  8:03                                 ` Visuwesh
2024-09-09 14:39                               ` Visuwesh
2024-09-14  9:43                                 ` Eli Zaretskii
2024-05-18 13:34           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-08 17:37       ` Jim Porter
2024-05-08 18:42         ` Eli Zaretskii
2024-05-08 19:19           ` Jim Porter
2024-05-08 19:23             ` Jim Porter
2024-05-09  4:41               ` Eli Zaretskii
2024-05-09 16:14                 ` Jim Porter

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.