Removing mode-specific window restore code

Removing mode-specific window restore code

[Christoph]

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

Some modes have their own quit function, which buries/kill buffer and often
times also restores the window configuration from its own internally stored
variables or runs specific cleanup code.

Previous discussion here:
https://lists.gnu.org/archive/html/emacs-devel/2011-10/msg00915.html

First, I would like to try to remove the mode-specific window restore code
and let `quit-window' handle this.

One low-hanging example (unless I am missing anything) is in ibuffer.el.

Using `quit-window' also allows to kill the buffer with `C-u q', which is
new behavior for `ibuffer', but align with other modes.

Next, I would like to look at modes that do "other stuff" in their
respective `quit' function. This goes back to the `quit-window-hook'
discussion in the above mentioned thread. This also includes modes that
just plainly call `bury-buffer' without restoring window configuration
instead of `quit-window' upon quitting the mode.

Should I open bug reports for each instance to track the review or post
patches here?

ibuffer patch, comments welcome:

=== modified file 'lisp/ibuffer.el'
--- lisp/ibuffer.el 2014-08-08 14:35:40 +0000
+++ lisp/ibuffer.el 2014-08-31 17:26:11 +0000
@@ -540,7 +540,7 @@
     (define-key map (kbd "/ X") 'ibuffer-delete-saved-filter-groups)
     (define-key map (kbd "/ \\") 'ibuffer-clear-filter-groups)

-    (define-key map (kbd "q") 'ibuffer-quit)
+    (define-key map (kbd "q") 'quit-window)
     (define-key map (kbd "h") 'describe-mode)
     (define-key map (kbd "?") 'describe-mode)

@@ -878,12 +878,6 @@
     (define-key map [down-mouse-3] 'ibuffer-mouse-popup-menu)
     map))

-(defvar ibuffer-restore-window-config-on-quit nil
-  "If non-nil, restore previous window configuration upon exiting
`ibuffer'.")
-
-(defvar ibuffer-prev-window-config nil
-  "Window configuration before starting Ibuffer.")
-
 (defvar ibuffer-did-modification nil)

 (defvar ibuffer-compiled-formats nil)
@@ -2298,18 +2292,6 @@
       (goto-char (point-min))
       (forward-line orig))))

-(defun ibuffer-quit ()
-  "Quit this `ibuffer' session.
-Try to restore the previous window configuration if
-`ibuffer-restore-window-config-on-quit' is non-nil."
-  (interactive)
-  (if ibuffer-restore-window-config-on-quit
-      (progn
- (bury-buffer)
- (unless (= (count-windows) 1)
-  (set-window-configuration ibuffer-prev-window-config)))
-    (bury-buffer)))
-
 ;;;###autoload
 (defun ibuffer-list-buffers (&optional files-only)
   "Display a list of buffers, in another window.
@@ -2350,7 +2332,6 @@
   (interactive "P")
   (when ibuffer-use-other-window
     (setq other-window-p t))
-  (setq ibuffer-prev-window-config (current-window-configuration))
   (let ((buf (get-buffer-create (or name "*Ibuffer*"))))
     (if other-window-p
  (funcall (if noselect (lambda (buf) (display-buffer buf t))
#'pop-to-buffer) buf)
@@ -2362,8 +2343,7 @@
  (select-window (get-buffer-window buf 0))
  (or (derived-mode-p 'ibuffer-mode)
     (ibuffer-mode))
- (setq ibuffer-restore-window-config-on-quit other-window-p)
- (when shrink
+ (when shrink
   (setq ibuffer-shrink-to-minimum-size shrink))
  (when qualifiers
   (require 'ibuf-ext)
@@ -2501,7 +2481,6 @@
   '\\[ibuffer-switch-format]' - Change the current display format.
   '\\[forward-line]' - Move point to the next line.
   '\\[previous-line]' - Move point to the previous line.
-  '\\[ibuffer-quit]' - Bury the Ibuffer buffer.
   '\\[describe-mode]' - This help.
   '\\[ibuffer-diff-with-file]' - View the differences between this buffer
           and its associated file.
@@ -2616,7 +2595,6 @@
   (set (make-local-variable 'ibuffer-cached-eliding-string) nil)
   (set (make-local-variable 'ibuffer-cached-elide-long-columns) nil)
   (set (make-local-variable 'ibuffer-current-format) nil)
-  (set (make-local-variable 'ibuffer-restore-window-config-on-quit) nil)
   (set (make-local-variable 'ibuffer-did-modification) nil)
   (set (make-local-variable 'ibuffer-tmp-hide-regexps) nil)
   (set (make-local-variable 'ibuffer-tmp-show-regexps) nil)

[-- Attachment #2: Type: text/html, Size: 6286 bytes --]

Re: Removing mode-specific window restore code

[Stefan Monnier]

> First, I would like to try to remove the mode-specific window restore code
> and let `quit-window' handle this.

Sounds good.  BTW, if you can make them derive from special-mode and
inherit the `q' binding from there, that'd be even better.

> Should I open bug reports for each instance to track the review or post
> patches here?

Better send patches as bug reports.

BTW, the Ibuffer patch looks fine,


        Stefan

Re: Removing mode-specific window restore code

[Christoph]

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

On Sun, Aug 31, 2014 at 2:05 PM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:


> Sounds good.  BTW, if you can make them derive from special-mode and
> inherit the `q' binding from there, that'd be even better.
>

OK.

Better send patches as bug reports.
>

OK.


> BTW, the Ibuffer patch looks fine,


Thanks for reviewing. Installed in trunk as r117790.

Christoph

[-- Attachment #2: Type: text/html, Size: 1075 bytes --]