bug#70820: [PATCH] Editable grep buffers

[Eli Zaretskii <eliz@gnu.org>]

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