tomterl / fullframe

Advice commands to execute fullscreen, restoring the window setup when exiting.
46 stars 4 forks source link

Handling certain exit functions #6

Closed purcell closed 10 years ago

purcell commented 10 years ago

I'd like to do something like this:

(fullframe list-packages quit-window :package-fullscreen nil)

but of course quit-window is a very commonly-bound command, and most of the time the advice runs, list-packages will not previously have been called. Can you think of a good way to handle this?

One option, of course, is to wrap up quit-window inside a new package-list-quit-window function, and bind that to q instead, but it seems like we could manage something better... :-)

tomterl commented 10 years ago

The easiest thing that comes to mind: we could use fullframe/--after-advice, set it in the command-on advice to register-name and check for it in the command-off advice, but as far as I can see, one advice would always match and execute jump-to-register -- we could just do the wrapping and keybinding ourselves in a macro, not? with an additional paramter key-binding that defaults to q.

purcell commented 10 years ago

Maybe, but that also seems quite heavy-handed: fullframe shouldn't know anything about key bindings.

Another idea: allow exit functions to be limited to the buffer created by the entry function, if an additional option is passed. So after list-packages runs, the current buffer is noted, and quit-window only unwinds the window configuration if the current buffer at that time matches the one noted.

tomterl commented 10 years ago

Could you check the following patch?

diff --git a/fullframe.el b/fullframe.el
index cc7f3cf..c7f7620 100644
--- a/fullframe.el
+++ b/fullframe.el
@@ -56,13 +56,13 @@

 ;; API
 ;;;###autoload
-(defmacro fullframe (command-on command-off register &optional kill-on-coff)
+(defmacro fullframe (command-on command-off register &optional kill-on-coff )
   "Save window/frame state when executing COMMAND-ON.

 Advice execution of command-on to store the current window until
-  COMMAND-OFF state in REGISTER and display a single
-  frame.  Advice COMMAND-OFF to restore the state stored in
-  REGISTER.  If KILL-ON-COFF is true, `kill-buffer' is called after
+  COMMAND-OFF state in REGISTER and display a single frame.
+  Advice COMMAND-OFF to restore the state stored in REGISTER.  If
+  KILL-ON-COFF is true, `kill-buffer' is called after
   command-off."
   (let* ((on-rule-name (cl-gensym "fullscreen-rule-"))
          (off-rule-name (cl-gensym "restore-setup-rule-"))
@@ -80,9 +80,10 @@ Advice execution of command-on to store the current window until
                (window-configuration-to-register ,register-name)
                ad-do-it
                (delete-other-windows)
-               (setq fullframe/--after-advice t))))
+               (setq fullframe/--after-advice (current-buffer)))))
        (defadvice ,command-off (after ,off-rule-name activate)
-         ,off-code))))
+         (if (eq fullframe/--after-advice (current-buffer))
+             ,off-code)))))

 ;; interactive functions
 ;; - none

It is buggy in my emacs, the minibuffer takes half the screen after exiting e.g. the magit window.

purcell commented 10 years ago

Ah... I'm just working on a different approach...

tomterl commented 10 years ago

Are you by any chance anywhere on freenode (#emacs perhaps)?

purcell commented 10 years ago

No, sorry, I haven't used IRC since about 1994. :-)

tomterl commented 10 years ago

Ah well, would have made communication easier and less intrusive for the masses interested in this piece of code :-) - If you use jabber I'm reachable as tom -at- sec [dot] goochesa [dot] de

purcell commented 10 years ago

Okay, see #7. If you'd prefer, I can prepare a version of the same patch which unconditionally checks the buffers, rather than adding the SAME-BUFFER parameter.

tomterl commented 10 years ago

I think for the sake of flexibility I'd like you to add a defcustom fullframe/always-same-buffer with a nil default - that acts as default for the same-buffer parameter to fullframe. What do you think?

purcell commented 10 years ago

Instead of the extra parameter, or in addition to it? I'm assuming you mean the latter.

tomterl commented 10 years ago

I'm on it - and yes the latter.

tomterl commented 10 years ago

does this look like it works for you?

diff --git a/fullframe.el b/fullframe.el
index 7f06d39..b493702 100644
--- a/fullframe.el
+++ b/fullframe.el
@@ -43,7 +43,13 @@
 (require 'cl-lib)

 ;; customization
-;; - none
+(defgroup fullframe ()
+  "Customizations to the fullframe package.")
+
+(defcustom fullframe/always-same-buffer nil
+  "When true, `same-buffer' of `fullframe' will default to true."
+  :group 'fullframe
+  :type 'boolean)

 ;; variables
 (defvar fullframe/--after-advice nil
@@ -76,7 +82,7 @@ Advice execution of command-on to store the current window until
                           (jump-to-register ,register-name))
                      `(jump-to-register ,register-name))))
     `(progn
-       (defvar ,allow-unwind-var (not ,same-buffer))
+       (defvar ,allow-unwind-var (not (or ,same-buffer fullframe/always-same-buffer)))
        (make-variable-buffer-local ',allow-unwind-var)
        (setq ,register-name ,register)
        (defadvice ,command-on (around ,on-rule-name activate)
purcell commented 10 years ago

I think that in this form, its behaviour may confuse users. Changing the custom var will have no effect after fullframe has been called, because the value is locked into the "allow-unwind-var" defvar at compile time.

What's the use case you've got in mind?

My feeling is that the defcustom won't really provide any benefit. And if the default value of same-buffer should really be t, which might be the case, then we can just rename it to any-buffer and change the code accordingly.

tomterl commented 10 years ago

I want users to be able to set their own default, and deviate from it for single cases. Makes sense or am I not thinking straight?

Steve Purcell notifications@github.com wrote:

I think that in this form, its behaviour may confuse users. Changing the custom var will have no effect after fullframe has been called, because the value is locked into the "allow-unwind-var" defvar at compile time.

What's the use case you've got in mind?

My feeling is that the defcustom won't really provide any benefit. And if the default value of same-buffer should really be t, which might be the case, then we can just rename it to any-buffer and change the code accordingly.


Reply to this email directly or view it on GitHub: https://github.com/tomterl/fullframe/issues/6#issuecomment-33480433

Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.

Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.

Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.

purcell commented 10 years ago

I reckon you're overthinking it: users need to write a little elisp to use fullframe anyway, so having a separate customizable var isn't a benefit. Between the two of us "proto-users", I think we can figure out what a reasonable default is, and let the users override it on a case-by-case basis.

tomterl commented 10 years ago

You're probably right. We should reverse the logic then, so that same frame is true by default.

Steve Purcell notifications@github.com wrote:

I reckon you're overthinking it: users need to write a little elisp to use fullframe anyway, so having a separate customizable var isn't a benefit. Between the two of us "proto-users", I think we can figure out what a reasonable default is, and let the users override it on a case-by-case basis.


Reply to this email directly or view it on GitHub: https://github.com/tomterl/fullframe/issues/6#issuecomment-33489059

Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.

purcell commented 10 years ago

Cool. Shall I send another PR?

tomterl commented 10 years ago

That would be most welcome. I'm away today from the keyboard for the most time.

Steve Purcell notifications@github.com wrote:

Cool. Shall I send another PR?


Reply to this email directly or view it on GitHub: https://github.com/tomterl/fullframe/issues/6#issuecomment-33494403

Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.