bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Leo]

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

I have extended the occur feature in Emacs to allow direct editing in
the *Occur* buffer by propagating the changes to the original buffers.

With the attached preliminary patch, one can press `C-x C-q' or `C-c
C-c' to enter occur-edit-mode and start editing. Pressing `C-x C-q' or
`C-c C-c' again finishes the edit.

Comments are highly welcomed. Thanks in advance.

Leo


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

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2011-04-08 03:05:58 +0000
+++ lisp/replace.el	2011-04-10 08:02:15 +0000
@@ -761,7 +761,8 @@
   (let ((map (make-sparse-keymap)))
     ;; We use this alternative name, so we can use \\[occur-mode-mouse-goto].
     (define-key map [mouse-2] 'occur-mode-mouse-goto)
-    (define-key map "\C-c\C-c" 'occur-mode-goto-occurrence)
+    (define-key map "\C-c\C-c" 'occur-edit-mode)
+    (define-key map "\C-x\C-q" 'occur-edit-mode)
     (define-key map "\C-m" 'occur-mode-goto-occurrence)
     (define-key map "o" 'occur-mode-goto-occurrence-other-window)
     (define-key map "\C-o" 'occur-mode-display-occurrence)
@@ -815,6 +816,18 @@
     map)
   "Keymap for `occur-mode'.")
 
+(defvar occur-edit-mode-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mouse-2] 'occur-mode-mouse-goto)
+    (define-key map "\C-c\C-c" 'occur-edit-mode-finish)
+    (define-key map "\C-x\C-q" 'occur-edit-mode-finish)
+    (define-key map "\C-m" 'occur-mode-goto-occurrence)
+    (define-key map "\C-o" 'occur-mode-display-occurrence)
+    (define-key map "\M-n" 'occur-next)
+    (define-key map "\M-p" 'occur-prev)
+    (define-key map "\C-c\C-f" 'next-error-follow-minor-mode)
+    map))
+
 (defvar occur-revert-arguments nil
   "Arguments to pass to `occur-1' to revert an Occur mode buffer.
 See `occur-revert-function'.")
@@ -849,6 +862,59 @@
   (add-hook 'change-major-mode-hook 'font-lock-defontify nil t)
   (setq next-error-function 'occur-next-error))
 
+(defun occur-edit-mode ()
+  (interactive)
+  (setq buffer-read-only nil)
+  (use-local-map occur-edit-mode-map)
+  (setq major-mode 'occur-edit-mode
+	mode-name "Occur-Edit")
+  (add-hook 'after-change-functions 'occur-after-change-function nil t)
+  (force-mode-line-update))
+
+(defvar occur-edit-modified-buffers nil)
+(make-variable-buffer-local 'occur-edit-modified-buffers)
+
+(defun occur-edit-mode-finish ()
+  (interactive)
+  (mapc (lambda (buf)
+	  (and (buffer-file-name buf)
+	       (with-current-buffer buf
+		 (save-buffer))))
+	occur-edit-modified-buffers)
+  (setq buffer-read-only t)
+  (kill-local-variable 'occur-edit-modified-buffers)
+  (remove-hook 'after-change-functions 'occur-after-change-function t)
+  (set-buffer-modified-p nil)
+  (use-local-map occur-mode-map)
+  (setq major-mode 'occur-mode
+	mode-name "Occur"))
+
+(defun occur-after-change-function (beg end length)
+  (let* ((m (get-text-property (point) 'occur-target))
+	 (buf (marker-buffer m))
+	 (col (current-column)))
+    (when (and (markerp m) (buffer-live-p buf))
+      (let ((line (- (line-number-at-pos)
+		     (line-number-at-pos (window-start))))
+	    (readonly (with-current-buffer buf buffer-read-only))
+	    (win (or (get-buffer-window buf)
+		     (display-buffer buf t)))
+	    (text (save-excursion
+		    (forward-line 0)
+		    (search-forward ":" nil t)
+		    (setq col (- col (current-column)))
+		    (buffer-substring-no-properties (point) (line-end-position)))))
+	(and (not readonly)
+	     (add-to-list 'occur-edit-modified-buffers buf))
+	(with-selected-window win
+	  (goto-char m)
+	  (recenter line)
+	  (if readonly
+	      (message "Buffer `%s' is read only." buf)
+	    (delete-region (line-beginning-position) (line-end-position))
+	    (insert text))
+	  (move-to-column col))))))
+
 (defun occur-revert-function (ignore1 ignore2)
   "Handle `revert-buffer' for Occur mode buffers."
   (apply 'occur-1 (append occur-revert-arguments (list (buffer-name)))))
@@ -1273,6 +1339,7 @@
 				      `(font-lock-face prefix-face))
 				    `(occur-prefix t mouse-face (highlight)
 						   occur-target ,marker follow-link t
+						   read-only t
 						   help-echo "mouse-2: go to this occurrence"))))
 			   (match-str
 			    ;; We don't put `mouse-face' on the newline,
@@ -1333,13 +1400,15 @@
 		(goto-char headerpt)
 		(let ((beg (point))
 		      end)
-		  (insert (format "%d match%s%s in buffer: %s\n"
-				  matches (if (= matches 1) "" "es")
-				  ;; Don't display regexp for multi-buffer.
-				  (if (> (length buffers) 1)
-				      "" (format " for \"%s\""
-						 (query-replace-descr regexp)))
-				  (buffer-name buf)))
+		  (insert (propertize
+			   (format "%d match%s%s in buffer: %s\n"
+				   matches (if (= matches 1) "" "es")
+				   ;; Don't display regexp for multi-buffer.
+				   (if (> (length buffers) 1)
+				       "" (format " for \"%s\""
+						  (query-replace-descr regexp)))
+				   (buffer-name buf))
+			   'read-only t))
 		  (setq end (point))
 		  (add-text-properties beg end
 				       (append

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Stefan Monnier]

> @@ -761,7 +761,8 @@
>    (let ((map (make-sparse-keymap)))
>      ;; We use this alternative name, so we can use \\[occur-mode-mouse-goto].
>      (define-key map [mouse-2] 'occur-mode-mouse-goto)
> -    (define-key map "\C-c\C-c" 'occur-mode-goto-occurrence)
> +    (define-key map "\C-c\C-c" 'occur-edit-mode)
> +    (define-key map "\C-x\C-q" 'occur-edit-mode)
>      (define-key map "\C-m" 'occur-mode-goto-occurrence)
>      (define-key map "o" 'occur-mode-goto-occurrence-other-window)
>      (define-key map "\C-o" 'occur-mode-display-occurrence)

I haven't had a chance to look much at the code, but while the
functionality generally sounds good, the C-c C-c binding above doesn't
look right to me.  I think C-x C-q is sufficient and there's no need to
introduce such an incompatibility (not with code but with users).


        Stefan

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Chong Yidong]

Leo <sdl.web@gmail.com> writes:

> I have extended the occur feature in Emacs to allow direct editing in
> the *Occur* buffer by propagating the changes to the original buffers.
>
> With the attached preliminary patch, one can press `C-x C-q' or `C-c
> C-c' to enter occur-edit-mode and start editing. Pressing `C-x C-q' or
> `C-c C-c' again finishes the edit.
>
> Comments are highly welcomed. Thanks in advance.

Looks good.  I've made a couple of tweaks, and have committed it to
trunk.  The C-c c-c binding was removed in favor of just C-x C-q, as
Stefan suggested.  For now, I also left out the part where C-x C-q in
Occur Edit mode saves the associated buffers, because this doesn't seem
to be properly thought out; what if the buffers have no associated
files?

Also, I added a small fix to occur-after-change-function to avoid
screwing up the Occur Edit buffer when multi-line text is inserted.
Since we can't create new Occur entries, this just ignores everything
after the inserted newline.  If you want to try and handle this properly
(i.e. by inserting a new Occur entry for each inserted line), go ahead.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Leo]

On 2011-05-29 07:05 +0800, Chong Yidong wrote:
[snipped 11 lines]
> Looks good.  I've made a couple of tweaks, and have committed it to
> trunk.  The C-c c-c binding was removed in favor of just C-x C-q, as
> Stefan suggested.  For now, I also left out the part where C-x C-q in
> Occur Edit mode saves the associated buffers, because this doesn't seem
> to be properly thought out; what if the buffers have no associated
> files?
>
> Also, I added a small fix to occur-after-change-function to avoid
> screwing up the Occur Edit buffer when multi-line text is inserted.
> Since we can't create new Occur entries, this just ignores everything
> after the inserted newline.  If you want to try and handle this properly
> (i.e. by inserting a new Occur entry for each inserted line), go ahead.

Yidong, many thanks for the tweaks and fixes.

Let's keep this bug open for a bit longer. I am thinking of generalising
occur-edit-mode (probably rename it to match-line-edit) so that it
covers buffers from grep and the like.

Leo

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Andrew W. Nosenko]

On Sun, Apr 10, 2011 at 11:14, Leo <sdl.web@gmail.com> wrote:
> I have extended the occur feature in Emacs to allow direct editing in
> the *Occur* buffer by propagating the changes to the original buffers.
>
> With the attached preliminary patch, one can press `C-x C-q' or `C-c
> C-c' to enter occur-edit-mode and start editing. Pressing `C-x C-q' or
> `C-c C-c' again finishes the edit.
>
> Comments are highly welcomed. Thanks in advance.

Is there a way to _disable_ this feature?  I'm quite often edit the
*Occur* and *grep* buffers for line-up results, etc, just for easier
understanding results, but I don't want these edits to be mirrored
into original lines or files from where they are come.

-- 
Andrew W. Nosenko <andrew.w.nosenko@gmail.com>

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Richard Stallman]

    > I have extended the occur feature in Emacs to allow direct editing in
    > the *Occur* buffer by propagating the changes to the original buffers.

I am not surprised you want to disable it.
Maybe it should be disabled by default.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org, www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Glenn Morris]

"Andrew W. Nosenko" wrote:

> Is there a way to _disable_ this feature?  I'm quite often edit the
> *Occur* and *grep* buffers for line-up results, etc, just for easier
> understanding results, but I don't want these edits to be mirrored
> into original lines or files from where they are come.

Me too.

I expect C-x C-q to toggle the read-only state of a buffer, not switch
me into some unusual mode. I suggest binding this new mode to some other
key.

Other issues with this:

1. After C-x C-q, I can no longer kill entire lines in the occur buffer.
Trying to do so reports "Text is read-only".

2. After C-x C-q, If I delete some text in the occur buffer, then use
"undo", when I reach the point at which there is no more to undo, I get:
"Wrong type argument: markerp, nil" rather than "No further undo information".

Debugger entered--Lisp error: (wrong-type-argument markerp nil)
  marker-buffer(nil)
  occur-after-change-function(1 40 39)
  primitive-undo(1 ((nil font-lock-face underline 1 . 40) (t 0 . 0)))
  undo-more(1)
  undo(nil)
  call-interactively(undo nil nil)

3. After C-x C-q, M-x revert-buffer in the occur buffer triggers an error:

apply: Wrong number of arguments: #[(regexp nlines bufs &optional buf-name)

  occur-1("*Occur*")
  apply(occur-1 "*Occur*")
  occur-revert-function(t nil)
  revert-buffer(t)
  call-interactively(revert-buffer t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)

I guess the mode change clobbers occur-revert-arguments.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Leo]

On 2011-06-02 07:03 +0800, Glenn Morris wrote:
[snipped 50 lines]

Thank you for this feedback. I will try to address these issues when I
am back from a trip on Tuesday.

Leo

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Stefan Monnier]

> Thank you for this feedback. I will try to address these issues when I
> am back from a trip on Tuesday.

I think it makes sense to bind the occur-edit feature to some other key
than C-x C-q.
While C-x C-q is a cute binding for it, it does hide a command that is
also useful.


        Stefan

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Thank you for this feedback. I will try to address these issues when I
>> am back from a trip on Tuesday.
>
> I think it makes sense to bind the occur-edit feature to some other
> key than C-x C-q.  While C-x C-q is a cute binding for it, it does
> hide a command that is also useful.

Unfortunately, C-x C-q is consistent with the command for switching to
wdired mode.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Štěpán Němec]

Chong Yidong <cyd@stupidchicken.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> Thank you for this feedback. I will try to address these issues when I
>>> am back from a trip on Tuesday.
>>
>> I think it makes sense to bind the occur-edit feature to some other
>> key than C-x C-q.  While C-x C-q is a cute binding for it, it does
>> hide a command that is also useful.
>
> Unfortunately, C-x C-q is consistent with the command for switching to
> wdired mode.

I don't think this argument is really valid. First of all, C-x C-q is
consistent with the command to switch read-only-ness of a buffer, and
that's apparently what at least some people still expect and are used to
in this case. It might have made sense to override it for Wdired, but
not necessarily here.

  Štěpán

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Stefan Monnier]

>>> Thank you for this feedback. I will try to address these issues when I
>>> am back from a trip on Tuesday.
>> I think it makes sense to bind the occur-edit feature to some other
>> key than C-x C-q.  While C-x C-q is a cute binding for it, it does
>> hide a command that is also useful.
> Unfortunately, C-x C-q is consistent with the command for switching to
> wdired mode.

Right, but wdired and occur-edit are two different things.
Editing a dired buffer in wdired does not have any other side effect
(at least not until you "commit" those changes), so its behavior is
still pretty close to toggle-read-only.  Occur-edit is somewhat
reminiscent of wdired, but does not match the behavior of
toggle-read-only quite as closely.


        Stefan

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Leo]

On 2011-06-02 07:03 +0800, Glenn Morris wrote:
> 2. After C-x C-q, If I delete some text in the occur buffer, then use
> "undo", when I reach the point at which there is no more to undo, I get:
> "Wrong type argument: markerp, nil" rather than "No further undo information".
>
> Debugger entered--Lisp error: (wrong-type-argument markerp nil)
>   marker-buffer(nil)
>   occur-after-change-function(1 40 39)
>   primitive-undo(1 ((nil font-lock-face underline 1 . 40) (t 0 . 0)))
>   undo-more(1)
>   undo(nil)
>   call-interactively(undo nil nil)

This can be fixed with:

diff --git a/lisp/replace.el b/lisp/replace.el
index 0578ed09..fc2db2ec 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -882,11 +882,11 @@ (define-derived-mode occur-edit-mode occur-mode "Occur-Edit"
 
 (defun occur-after-change-function (beg end length)
   (save-excursion
-    (goto-char beg)
     (let* ((m (get-text-property (line-beginning-position) 'occur-target))
 	   (buf (marker-buffer m))
 	   (col (current-column)))
       (when (= length 0)
+	(goto-char beg)
 	;; Apply occur-target property to inserted (e.g. yanked) text.
 	(put-text-property beg end 'occur-target m)
 	;; Did we insert a newline?  Occur Edit mode can't create new

The cause:

When switching major-mode (in this case occur-edit-mode), an undo entry
containing text-property only change is added due to this line in
occur-mode:

(add-hook 'change-major-mode-hook 'font-lock-defontify nil t)

That line seems useless due to change made in revid 22063400b. If no one
objects, I intend to also remove it.

Leo

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Glenn Morris]

Leo wrote:

> revid 22063400b.

Speaking for myself, I have no idea what that means.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Leo]

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

I think maybe we should assign C-c C-c to occur-edit-mode instead.

   1. After C-x C-q, I can no longer kill entire lines in the occur buffer.
   Trying to do so reports "Text is read-only".

The text properties are now added when entering occur-edit-mode.

   2. After C-x C-q, If I delete some text in the occur buffer, then use
   "undo", when I reach the point at which there is no more to undo, I get:
   "Wrong type argument: markerp, nil" rather than "No further undo information".

   Debugger entered--Lisp error: (wrong-type-argument markerp nil)
     marker-buffer(nil)
     occur-after-change-function(1 40 39)
     primitive-undo(1 ((nil font-lock-face underline 1 . 40) (t 0 . 0)))
     undo-more(1)
     undo(nil)
     call-interactively(undo nil nil)

Fixed.

   3. After C-x C-q, M-x revert-buffer in the occur buffer triggers an error

occur-revert-arguments is killed after any major-mode change. In the
patch this has been made permanent-local.

Comments welcome. Thanks.


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

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2011-05-28 22:56:14 +0000
+++ lisp/replace.el	2011-06-09 09:28:09 +0000
@@ -826,6 +826,8 @@
 (defvar occur-revert-arguments nil
   "Arguments to pass to `occur-1' to revert an Occur mode buffer.
 See `occur-revert-function'.")
+(make-variable-buffer-local 'occur-revert-arguments)
+(put 'occur-revert-arguments 'permanent-local t)
 
 (defcustom occur-mode-hook '(turn-on-font-lock)
   "Hook run when entering Occur mode."
@@ -853,8 +855,6 @@
 
 \\{occur-mode-map}"
   (set (make-local-variable 'revert-buffer-function) 'occur-revert-function)
-  (make-local-variable 'occur-revert-arguments)
-  (add-hook 'change-major-mode-hook 'font-lock-defontify nil t)
   (setq next-error-function 'occur-next-error))
 
 \f
@@ -865,7 +865,7 @@
     (set-keymap-parent map text-mode-map)
     (define-key map [mouse-2] 'occur-mode-mouse-goto)
     (define-key map "\C-c\C-c" 'occur-mode-goto-occurrence)
-    (define-key map "\C-x\C-q" 'occur-mode)
+    (define-key map "\C-x\C-q" 'occur-edit-finish)
     (define-key map "\C-c\C-f" 'next-error-follow-minor-mode)
     (define-key map [menu-bar occur] (cons (purecopy "Occur") occur-menu-map))
     map)
@@ -878,40 +878,59 @@
 
 To return to ordinary Occur mode, use \\[occur-mode]."
   (setq buffer-read-only nil)
+  (put 'occur-noneditable 'read-only t)
+  (put 'occur-noneditable 'face 'shadow)
+  (save-excursion
+    (save-restriction
+      (widen)
+      (goto-char (point-min))
+      (while (re-search-forward "^[ \t0-9]+:\\|\n" nil t)
+	(with-silent-modifications
+	  (put-text-property (match-beginning 0) (match-end 0)
+			     'category 'occur-noneditable)))))
   (add-hook 'after-change-functions 'occur-after-change-function nil t))
 
+(defun occur-edit-finish ()
+  (interactive)
+  (put 'occur-noneditable 'read-only nil)
+  (put 'occur-noneditable 'face nil)
+  (occur-mode))
+
 (defun occur-after-change-function (beg end length)
-  (save-excursion
-    (goto-char beg)
-    (let* ((m (get-text-property (line-beginning-position) 'occur-target))
-	   (buf (marker-buffer m))
-	   (col (current-column)))
-      (when (= length 0)
-	;; Apply occur-target property to inserted (e.g. yanked) text.
-	(put-text-property beg end 'occur-target m)
-	;; Did we insert a newline?  Occur Edit mode can't create new
-	;; Occur entries; just discard everything after the newline.
+  (let ((m (save-excursion
+	     (goto-char beg)
+	     (get-text-property (line-beginning-position) 'occur-target))))
+    (when (and (markerp m) (buffer-live-p (marker-buffer m)))
+      (when (zerop length)
 	(save-excursion
+	  ;; Apply occur-target property to inserted (e.g. yanked) text.
+	  (put-text-property beg end 'occur-target m)
+	  ;; Did we insert a newline?  Occur Edit mode can't create new
+	  ;; Occur entries; just discard everything after the newline.
+	  (goto-char beg)
 	  (and (search-forward "\n" end t)
 	       (delete-region (1- (point)) end))))
-      (let ((line (- (line-number-at-pos)
-		     (line-number-at-pos (window-start))))
-	    (readonly (with-current-buffer buf buffer-read-only))
-	    (win (or (get-buffer-window buf)
-		     (display-buffer buf t)))
-	    (text (save-excursion
-		    (forward-line 0)
-		    (search-forward ":" nil t)
-		    (setq col (- col (current-column)))
-		    (buffer-substring-no-properties (point) (line-end-position)))))
-	(with-selected-window win
-	  (goto-char m)
-	  (recenter line)
-	  (if readonly
-	      (message "Buffer `%s' is read only." buf)
-	    (delete-region (line-beginning-position) (line-end-position))
-	    (insert text))
-	  (move-to-column col))))))
+      (save-excursion
+	(let* ((buf (marker-buffer m))
+	       (col (current-column))
+	       (line (- (line-number-at-pos)
+			(line-number-at-pos (window-start))))
+	       (readonly (with-current-buffer buf buffer-read-only))
+	       (win (or (get-buffer-window buf)
+			(display-buffer buf t)))
+	       (text (save-excursion
+		       (forward-line 0)
+		       (search-forward ":" nil t)
+		       (setq col (- col (current-column)))
+		       (buffer-substring-no-properties (point) (line-end-position)))))
+	  (with-selected-window win
+	    (goto-char m)
+	    (recenter line)
+	    (if readonly
+		(message "Buffer `%s' is read only." buf)
+	      (delete-region (line-beginning-position) (line-end-position))
+	      (insert text))
+	    (move-to-column col)))))))
 
 \f
 (defun occur-revert-function (_ignore1 _ignore2)
@@ -1341,7 +1360,6 @@
 				      `(font-lock-face prefix-face))
 				    `(occur-prefix t mouse-face (highlight)
 						   occur-target ,marker follow-link t
-						   read-only t
 						   help-echo "mouse-2: go to this occurrence"))))
 			   (match-str
 			    ;; We don't put `mouse-face' on the newline,
@@ -1401,15 +1419,13 @@
 		(goto-char headerpt)
 		(let ((beg (point))
 		      end)
-		  (insert (propertize
-			   (format "%d match%s%s in buffer: %s\n"
-				   matches (if (= matches 1) "" "es")
-				   ;; Don't display regexp for multi-buffer.
-				   (if (> (length buffers) 1)
-				       "" (format " for \"%s\""
-						  (query-replace-descr regexp)))
-				   (buffer-name buf))
-			   'read-only t))
+		  (insert (format "%d match%s%s in buffer: %s\n"
+				  matches (if (= matches 1) "" "es")
+				  ;; Don't display regexp for multi-buffer.
+				  (if (> (length buffers) 1)
+				      "" (format " for \"%s\""
+						 (query-replace-descr regexp)))
+				  (buffer-name buf)))
 		  (setq end (point))
 		  (add-text-properties beg end
 				       (append

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Leo]

On 2011-06-09 13:14 +0800, Glenn Morris wrote:
>> revid 22063400b.
>
> Speaking for myself, I have no idea what that means.

That's what I saw on the annotation buffer (from C-x v g). I was able to
view the diff there with 'd'.

Leo

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Eli Zaretskii]

> From: Leo <sdl.web@gmail.com>
> Date: Thu, 09 Jun 2011 17:44:11 +0800
> Cc: 8463@debbugs.gnu.org
> 
> On 2011-06-09 13:14 +0800, Glenn Morris wrote:
> >> revid 22063400b.
> >
> > Speaking for myself, I have no idea what that means.
> 
> That's what I saw on the annotation buffer (from C-x v g). I was able to
> view the diff there with 'd'.

Git sha values are generally not useful for someone who uses bzr.  It
would be nice if people who report problems could also mention the bzr
revision or revid.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Leo]

On 2011-06-09 17:54 +0800, Eli Zaretskii wrote:
>> That's what I saw on the annotation buffer (from C-x v g). I was able to
>> view the diff there with 'd'.
>
> Git sha values are generally not useful for someone who uses bzr.  It
> would be nice if people who report problems could also mention the bzr
> revision or revid.

My bad. I thought I was on the bzr repo.

Leo

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Glenn Morris]

Leo wrote:

> I think maybe we should assign C-c C-c to occur-edit-mode instead.

Or "e" (for edit; like in Rmail).

>    1. After C-x C-q, I can no longer kill entire lines in the occur buffer.
>    Trying to do so reports "Text is read-only".
>
> The text properties are now added when entering occur-edit-mode.

I don't understand if that means it's fixed. Deleting whole or even
multiple lines is an operation I might want to do in occur-edit-mode.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Stefan Monnier]

>> I think maybe we should assign C-c C-c to occur-edit-mode instead.

I don't like that: C-c C-c means "done" usually so it'd be OK to leave
edit mode, but not to enter it.

> Or "e" (for edit; like in Rmail).

That sounds good.

>> 1. After C-x C-q, I can no longer kill entire lines in the occur buffer.
>> Trying to do so reports "Text is read-only".
>> The text properties are now added when entering occur-edit-mode.
> I don't understand if that means it's fixed. Deleting whole or even
> multiple lines is an operation I might want to do in occur-edit-mode.

But its meaning is unclear and we get to choose what is allowed and what
is not in edit mode since it's a new mode that the user
requested explicitly.


        Stefan

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Ted Zlatanov]

On Fri, 10 Jun 2011 10:59:16 -0300 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>>> 1. After C-x C-q, I can no longer kill entire lines in the occur buffer.
>>> Trying to do so reports "Text is read-only".
>>> The text properties are now added when entering occur-edit-mode.
>> I don't understand if that means it's fixed. Deleting whole or even
>> multiple lines is an operation I might want to do in occur-edit-mode.

SM> But its meaning is unclear and we get to choose what is allowed and what
SM> is not in edit mode since it's a new mode that the user
SM> requested explicitly.

I'd suggest binding `d' to "delete entire line" outside the edit mode,
so the edit mode does not need to provide it and still it's available in
a convenient way.

Ted

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Andrew W. Nosenko]

On Fri, Jun 10, 2011 at 16:59, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> I think maybe we should assign C-c C-c to occur-edit-mode instead.
>
> I don't like that: C-c C-c means "done" usually so it'd be OK to leave
> edit mode, but not to enter it.
>
>> Or "e" (for edit; like in Rmail).
>
> That sounds good.

But not for case, when user already switched the *Occur* buffer into
read/write mode for editing the buffer itself (not the underlying
files/buffers, where occurrences come from).

The same applied for Ted's proposal for 'd' key and any other keys
that used for regular text typing.

-- 
Andrew W. Nosenko <andrew.w.nosenko@gmail.com>

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Juri Linkov]

>>> Or "e" (for edit; like in Rmail).
>>
>> That sounds good.
>
> But not for case, when user already switched the *Occur* buffer into
> read/write mode for editing the buffer itself (not the underlying
> files/buffers, where occurrences come from).

Then maybe `M-e' (like in isearch mode)?

> The same applied for Ted's proposal for 'd' key and any other keys
> that used for regular text typing.

There is already `C-k' to do this.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Andrew W. Nosenko]

On Sat, Jun 11, 2011 at 21:00, Juri Linkov <juri@jurta.org> wrote:
>>>> Or "e" (for edit; like in Rmail).
>>>
>>> That sounds good.
>>
>> But not for case, when user already switched the *Occur* buffer into
>> read/write mode for editing the buffer itself (not the underlying
>> files/buffers, where occurrences come from).
>
> Then maybe `M-e' (like in isearch mode)?

If separate apply-changes-to-underlying-buffers action to the own
function (may be even it already implemented in that way) then all
controversy will gone. -- Enter to read-write mode -- the same,
editing commands -- the same, all other behavior -- the same.  Just
because there will work one and the same code in the one and the same
mode.  Difference is only one between my usage model and usage model
of Leo (original author) -- after finishing I simple kill the occur
buffer, while Leo call the "apply-cachanges-to-underlying-buffers" for
"commit" the final results.

IMHO anyone can benefit from solving the problem in that way.

-- 
Andrew W. Nosenko <andrew.w.nosenko@gmail.com>

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Chong Yidong]

"Andrew W. Nosenko" <andrew.w.nosenko@gmail.com> writes:

>>> Or "e" (for edit; like in Rmail).
>>
>> That sounds good.
>
> But not for case, when user already switched the *Occur* buffer into
> read/write mode for editing the buffer itself (not the underlying
> files/buffers, where occurrences come from).

That's already the case with "r" (occur-rename-buffer), "c"
(clone-buffer), and "o" (occur-mode-goto-occurrence-other-window).

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Andrew W. Nosenko]

On Sat, Jun 18, 2011 at 22:35, Chong Yidong <cyd@stupidchicken.com> wrote:
> "Andrew W. Nosenko" <andrew.w.nosenko@gmail.com> writes:
>
>>>> Or "e" (for edit; like in Rmail).
>>>
>>> That sounds good.
>>
>> But not for case, when user already switched the *Occur* buffer into
>> read/write mode for editing the buffer itself (not the underlying
>> files/buffers, where occurrences come from).
>
> That's already the case with "r" (occur-rename-buffer), "c"
> (clone-buffer), and "o" (occur-mode-goto-occurrence-other-window).
>

Yes, but: (1) these actions aren't destructive in respect to the
original data, and (2) I would to prefer to see the number of such
bindings decreasing, not increasing.

-- 
Andrew W. Nosenko <andrew.w.nosenko@gmail.com>

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Juri Linkov]

I appreciate this new feature Occur-Edit.  It's especially powerful with
the ability of syncing replacements made by `query-replace' from the
*Occur* buffer to the source buffer.  It could provide a special
keybinding to start making replacements with the same regexp as used to run
`occur' like grep-x.el does in http://thread.gmane.org/gmane.emacs.devel/66418
and in http://thread.gmane.org/gmane.emacs.devel/108202/focus=108229

Also it has one little drawback: it adds too many undo boundaries,
so to undo changes in the source buffer, it requires too many `C-_'.
There is no such problem in all.el (from ELPA) because
`all-after-change-function' doesn't call `recenter' and `move-to-column'.

But there is a greater drawback: stealing the `C-x C-q' keybinding from
`toggle-read-only' and putting read-only properties on the *Occur* buffer
is unacceptable.  I'm used to run `flush-lines' and `keep-lines' to narrow
the search results, but can't do this anymore.  This is a regression.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Juri Linkov]

There is 2011-06-18 ChangeLog entry added by revno:104628 without actual
code changes:

	* replace.el (occur-mode-map): Set occur-edit-mode binding to "e".

I guess it was supposed to bind "e" to `occur-edit-mode' in `occur-mode-map'?

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Chong Yidong]

Juri Linkov <juri@jurta.org> writes:

> But there is a greater drawback: stealing the `C-x C-q' keybinding
> from `toggle-read-only' and putting read-only properties on the
> *Occur* buffer is unacceptable.  I'm used to run `flush-lines' and
> `keep-lines' to narrow the search results, but can't do this anymore.
> This is a regression.

OK, I rebound occur-edit-mode to "e" and removed the read-only
properties, with some additional code changes to make Occur Edit mode
work without requiring read-only text.

As for the other issue of single-char commands not being usable in
non-read-only Occur mode, we could handle it in a way similar to diff
mode (via minor-mode-overriding-map-alist), but that is for another day.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Juri Linkov]

> OK, I rebound occur-edit-mode to "e" and removed the read-only
> properties, with some additional code changes to make Occur Edit mode
> work without requiring read-only text.

Thanks, this is fine now except one little problem:
after finishing Occur-Edit, `g' (`revert-buffer') fails with
"Wrong number of arguments: #[(regexp nlines bufs &optional buf-name)..."

There is a fix in http://debbugs.gnu.org/8463#47
that makes `occur-revert-arguments' permanent-local.

bug#8463: 24.0.50; [PATCH] Direct Edit in *Occur* Buffer

[Chong Yidong]

Juri Linkov <juri@jurta.org> writes:

> Thanks, this is fine now except one little problem:
> after finishing Occur-Edit, `g' (`revert-buffer') fails with
> "Wrong number of arguments: #[(regexp nlines bufs &optional buf-name)..."
>
> There is a fix in http://debbugs.gnu.org/8463#47
> that makes `occur-revert-arguments' permanent-local.

Ah, I somehow missed that patch.  I will incorporate it.  Thanks.