Overlay other windows, fixes, stack management and more...

[mehw]

Hello.

Your work is so useful that I couldn't resist to study the code!

I did some modding to put overlays over the other windows rather than the current one. There are also some fixes and new ideas like a different stack management...

I hope you could provide some feedbacks.

PS: I'm still debugging the code, there are some additions I didn't push yet, like customizable bindings and aliases. I didn't update the readme, nor the tests...

Best regards,
Matthew

[dpsutton]

@mehw nice work! I'm really liking it and it looks really good for the codebase. I've applied your patch locally to test it and have a few notes:

• the use of timers might not be the best. I had added ivy-buffer-list as shortcut ?l and i reflexively do this very quickly. The timer display pops up after i've invoked an action and obscures my buffer list. Need to inhibit the timers once an action is selected.
• unrecognized characters display a message showing they are unrecognized. I've gotten into the habit of using enter or random characters that i know won't match to exit. I'm not sure I want to change this behavior.

[dpsutton]

Also need to make sure that the tests pass. I need to update the matrix of which emacs it runs in. Currently its set to emacs 24 :(

[mehw]

@dpsutton I'm happy you enjoy the code!

• I see what you mean by ivy-buffer-list... messages are replaced by the timed resize-window's status. What about making it optional via defcustom? A better solution is to implement a console, a side window dedicated to resize-window that handles both the help menu and status messages... This could be another hint that resize-window is active. What do you think?
• resize-window-unregistered-key-quit could be another defcustom. I'm also implementing customizable bindings and aliases... what about a quit function the user could bind to chosen keys ?
• I'll take care of the tests as much as I can... may I ask for help to you If I'm in trouble?

I'll push the modifications as new commits, but once you agree the code is ready, maybe it's the case to squash the fixes to keep the branch clean. What do you think?

[dpsutton]

may I ask for help to you If I'm in trouble?

Absolutely! Happy to help and I really appreciate the work you have already done.

About squashing, I'm not sure squashing brings any benefit. Your commits are atomic and descriptive. I see no reason to smash them into one. I'm happy to accept the PR with as many commits as it takes.

Can you tell me what benefit the timers give? Why not just use messages so that the last one wins? Ie, if resize-window is outputting some message display that. But if whatever you have invoked is showing something, don't worry about timing and interfering? I don't see what benefit they bring yet. If you could explain that it would be helpful.

The defcustom for whether to quit or not on unrecognized keys sounds like a good idea. You were more recently a new user so you could set the default behavior. Perhaps people new to it would prefer the warning that the key they hit was unrecognized? So the default behavior is to not quit but warn?

[mehw]

Absolutely! Happy to help and I really appreciate the work you have already done.

Thanks!

About squashing, I'm not sure squashing brings any benefit. Your commits are atomic and descriptive. I see no reason to smash them into one.

Sorry, let me clarify. In this PR, squash each future fix with the commit it fixes. When you agree, a forced push could keep the history of this branch clean until it is ready to be merged into master...

Can you tell me what benefit the timers give? Why not just use messages so that the last one wins?

The timer allows to show the resize-window's status "Resize mode: insert KEY, ? for help, q or SPACE to quit" again after printing an action's message. This is also a hint to the fact that resize-window is still active. It is not essential, though. I was thinking more about implementing a console (side window) to handle both the help menu and status messages...

The defcustom for whether to quit or not on unrecognized keys sounds like a good idea. You were more recently a new user so you could set the default behavior. Perhaps people new to it would prefer the warning that the key they hit was unrecognized? So the default behavior is to not quit but warn?

Yes ...Warn that the key is unrecognized but give the possibility to use any unrecognized key to quit.

[dpsutton]

@mehw I've been looking over it and I'm quite pleased. You've certainly done a lot to improve the codebase.

I have a suggestion for displaying the timers:

(defun resize-window--notify-status ()
  "Display status message."
  (when (and resize-window-notify-with-messages
             (not (minibuffer-window-active-p (selected-window))))
    (message "Resize mode: insert KEY, ? for help, q or SPACE to quit")))

or a bit fancier

(defun resize-window--notify-status ()
  "Display status message."
  (when resize-window-notify-with-messages
    (if (minibuffer-window-active-p (selected-window))
        (push (run-with-timer 1.5 nil #'resize-window--notify-status)
              resize-window--notify-timers)
     (message "Resize mode: insert KEY, ? for help, q or SPACE to quit"))))

we can just spinloop the notifications for some time to come back when the minibuffer is no longer active.

You surprised me by moving the command that was under r but i'll adapt :)

I think there is only one blocker remaining and that is to put a way to quit on unrecognized input. It's quite hard for me to use right now as I've grown accustomed to double tapping enter in dired buffers and C-n in regular buffers and its quite jarring to me. Give me a defcustom so I can turn this off and I think we are good to go

Inhibiting when the minibuffer is active should suffice for my needs.

[dpsutton]

oh also i updated the test matrix so if you rebase on master it should give a more modern array of emacsen to test against. I would like the tests to pass before merging but i can clean that up after if need be. Again thanks so much for making software that I use every day much better.

[dpsutton]

this doesn't call resize-window--cancel-notify like when a [?q] input is read. Is this deliberate? Seems like cleanup should largely be consistent.

[mehw]

You just spot a bug! I'll fix it.

[mehw]

Fixed in forced push.

[dpsutton]

oh and one other thing: you certainly deserve a spot as one of the authors now. So I'll invite you to add your name into the header of the file if you wish.

[mehw]

we can just spinloop the notifications for some time to come back when the minibuffer is no longer active.

That is really a good idea! Great thinking!

You surprised me by moving the command that was under r but i'll adapt :)

I'm still debugging a move forward/backward in the stack. While I was coding, r and R are the first bindings I thought about... We could change the bindings to a more suitable configuration before merging into master. There's also another addition I'm working on to make bindings and aliases customizable...

I think there is only one blocker remaining and that is to put a way to quit on unrecognized input.

Got it!

Inhibiting when the minibuffer is active should suffice for my needs.

Are you talking about the notification mod you suggested?

oh also i updated the test matrix so if you rebase on master it should give a more modern array of emacsen to test against.

I'll do a forced push of the rebased branch.

I would like the tests to pass before merging but i can clean that up after if need be.

I see you use ert and undercover. Could you give me some hints on how to run the tests locally? This is going to help me a lot to update the tests!

Again thanks so much for making software that I use every day much better.

Well... I shall thank you for making this software in the first place...!

oh and one other thing: you certainly deserve a spot as one of the authors now. So I'll invite you to add your name into the header of the file if you wish.

That's a honor! Thanks!

I'll push the rebased branch and your suggestions soon. I'm on it right now.

[mehw]

Forced push done:

• Rebase on master.
• Update readme.md across commits: git log --patch readme.md
• Fix commit Save state command:
  • Add s command to comments.
• Fix commit Persistent status message by timeout:
  • Spinloop the status notification if the minibuffer is active.
  • Add resize-window--cancel-notify to quit condition case.
• New commits:
  • Customizable option to allow to quit on unregistered key
  • Update copyright notice
  • Update license link in readme.md

TODO:

• Update tests!
• Move forward/backward in the stack (r and R).
• Customizable bindings and aliases.

Coding right now...

[mehw]

I took some time to look at Cask...

Forced push:

• Update tests: git log --patch test/resize-window-test.el
• Fix commit Capture key combinations like "M-w":
  • Convert capital key sequence to string.

TODO:

• More tests.
• Move forward/backward in the stack (r and R).
• Customizable bindings and aliases.

Travis fails on emacs:

• emacs-25.3: No such package: emacs-25.3-bin
• emacs-26.1: No such package: emacs-26.1-bin
• emacs-git-snapshot: configure: error: You do not seem to have makeinfo >= 4.13

@dpsutton I see that previously on Travis some emacs-24 revisions were failing on Symbol's function definition is void: assoc-if due to a missing alias to cl-assoc-if. Since you updated the emacs test matrix, this should no longer be the case... right?!

Back to coding...

[dpsutton]

Are you talking about the notification mod you suggested?

I totally misspoke above. The only thing that I need to happen is a way to stop the event loop on unrecognized input.

running tests

Cask is what i use and it is quite simple to get it running locally:

$ cask install

$ make unit

Navigating the window stack

I originally used k and y. K came from "kill" and y from "yank" so it would mimic the kill-ring buffer. Using a lower case and capital might throw some complications since capital letters are assumed to be a "big" movement of a lowercase. Note that f and F do the same thing moving the window border forward by 1 and 5, respectively. If someone was to use resize-window-swap-capital-and-lowercase-behavior like I do it would invert these but that is probably not the intended behavior.

Test Runner

I'll look into this but the feedback loop is slow. I might wait until after your patch to poke it. Just run the tests locally using make unit and it should be fine.

[dpsutton]

I believe i have solved the testing issue if you rebase on master again.

[mehw]

The only thing that I need to happen is a way to stop the event loop on unrecognized input.

Could resize-window-unregistered-key-quit be a possible answer?

Cask is what i use and it is quite simple to get it running locally.

Thanks for the explanation!

I originally used k and y. K came from "kill" and y from "yank" so it would mimic the kill-ring buffer. Using a lower case and capital might throw some complications since capital letters are assumed to be a "big" movement of a lowercase. Note that f and F do the same thing moving the window border forward by 1 and 5, respectively. If someone was to use resize-window-swap-capital-and-lowercase-behavior like I do it would invert these but that is probably not the intended behavior.

I'm implementing forward/backward stack movements. Do you have any suggestion beside r and R as key bindings? I took inspiration from w and W which were already implemented to cycle windows forward/backward. IMHO resize-window-swap-capital-and-lowercase-behavior shouldn't be a concern when resize-window--allows-capitals returns nil, which means that in resize-window-dispatch-alist the key binding last value is nil (only non-nil, i.e. t, enables the capital counterpart).

I'll look into this but the feedback loop is slow. I might wait until after your patch to poke it. Just run the tests locally using make unit and it should be fine.

You missed me... what happened to the feedback loop?

I believe i have solved the testing issue if you rebase on master again.

This is odd, Travis emacs-25.3 doesn't have cl-assoc-if aliased to assoc-if (Symbol's function definition is void: assoc-if)... I fixed that in commit Capture key combinations like "M-w" by assoc-if -> cl-assoc-if, I also did a find-if-not -> cl-find-if-not.

There are still some Travis failures:

• emacs-26.1: configure: error: You do not seem to have makeinfo >= 4.13
• emacs-git-snapshot: configure: error: You do not seem to have makeinfo >= 4.13

[mehw]

Updated .travis.yml to install latest texinfo. This fixes configure: error: You do not seem to have makeinfo >= 4.13:

before_install:
  - curl -fsSkL https://gist.github.com/rejeep/7736123/raw > travis.sh && source ./travis.sh
  - sudo apt-get update
  - sudo apt-get install -y texinfo
  - evm install $EVM_EMACS --use --skip
  - cask

@dpsutton emacs-git-snapshot hangs up on cask for unknown reasons and Travis fails...

[mehw]

Currently working on forward/backward stack movements... I'm trying different behaviours...

[mehw]

@dpsutton I'm pushing the latest commit for review...

Forced push:

• New commit Stack forward/backward movements.
• Fix commit Window configurations stack management:
  • Update docstrings.
  • Set resize-window--window-stack to nbutlast result.
• Fix commit Reorganize the list of commands:
  • Revert back to y as binding to restore a state.
  • In resize-window-dispatch-alist "Restore state" -> "Restore state (save state)".

TODO:

• More tests.
• Code reviewing and debugging...
• Customizable bindings and aliases.

The window configurations stack

The stack is a customizable size holder for window configurations. It folds over. Moving after the end restarts from the beginning and vice versa. Old configurations are dropped due to a chosen reduction in its size or an exceding number of configurations saved.

Move forward/backward via > and < (to avoid pressing a modifier key, you may consider , and . as possible alternatives). Originally I was using r and R to move in the stack... @dpsutton what do you suggest?

Special flags give hints about the direction followed, forward > or backward <, and if the current window configuration is modified * or not = (aka saved in the stack at the current position).

When a configuration is modified, adjacent positions in the stack are considered to see if such new configuration is already there. In such a case, modification flag and direction followed are set accordingly.

[dpsutton]

@mehw I've run into a show stopper that needs to be addressed. Using resize-window will alter the window layout of other frames.

For instance, I have two frames open, both just split down the middle. Switching windows in one frame will alter the window layout in the other frame. Usually the split is gone and I'm left with only window in the other frame when I switch to it.

I'm guessing this is coming from the restoring the window configuration all the time and comparing it. I'm not sure I follow all of the care going through in that.

[dpsutton]

Also, just to be clear about my priorities:

I really like the cleanup and moving overlays to other windows. There's a lot of much needed cleanup in there.

I'm a little worried that the stack browser of window configs might impose a bit more cognitive overhead on the user than i initially intended. Also, the mechanism at the moment seems to mutate and destroy other window's current configuration which can't work.

Is there any way to merge in the current change with all of the window config stuff backed out and start a new PR for that when its more polished? At the moment this has ballooned into just a development branch rather than a feature branch.

[mehw]

I've run into a show stopper that needs to be addressed. Using resize-window will alter the window layout of other frames.

@dpsutton Thanks for spotting that!

resize-window is currently using a single stack for all frames, also it allows frames to operate unrestricted on other frames... Any frame is able to restore the configuration of another frame, and doing so this will modify the layout of the frame the configuration belongs to.

For instance, I have two frames open, both just split down the middle. Switching windows in one frame will alter the window layout in the other frame. Usually the split is gone and I'm left with only window in the other frame when I switch to it.

In the graphical version of Emacs, try to place two frames side by side. The resize-window's status messages should appear at the bottom of the frame the restored configuration belongs to... In the terminal version emacs -nw there's only one frame that handles virtual sub-frames, you'll see the frame label changing when restoring a configuration of another frame.

I'm guessing this is coming from the restoring the window configuration all the time and comparing it. I'm not sure I follow all of the care going through in that.

You get the same side effects from the master branch... It's not about refreshing/comparing the configurations, it is the way the stack is handled, unrestrictedly. Maybe you didn't step into this behaviour before cuz the implementation of the stack was just to pop configurations from it and forget them... but you'll land into this kind of chaotic behaviour eventually...

Anyway, resize-window can be made frame-restricted (aka operate on the current frame only). Already working is when a frame is deleted, the next run of resize-window will tidy up the stack of obsolete configurations.

I'm a little worried that the stack browser of window configs might impose a bit more cognitive overhead on the user than i initially intended. Also, the mechanism at the moment seems to mutate and destroy other window's current configuration which can't work.

IMHO just restricting resize-window to operate only on the frame it was run from should clear the confusion... It's unexpected seeing the neighbouring frames' layout changing while operating from another frame... It is not the stack management that introduced this...!

Is there any way to merge in the current change with all of the window config stuff backed out and start a new PR for that when its more polished? At the moment this has ballooned into just a development branch rather than a feature branch.

Sure! We loose the stack management... The master branch still need to be fixed though, or these kind of problems will keep lurking...

I'm working on the code right now... debugging...

[dpsutton]

@mehw I think i was unclear. When I have two frames open, both with split window configurations, I get window changes and buffers appear even when I'm not doing any stack options. I can recreate this just by cycling through windows with w and by invoking ivy-switch-buffer. I understand window config spans across frames and I am fine with that. The problem i am seeing is that I am having buffers appear and disappear just from cycling through windows or changing buffers in another frame.

Does that make sense?

[mehw]

Forced push:

• New commit Drop current configuration from the stack.
• Fix commit Stack forward/backward movements:
  • Reword commit comment.
  • More stack utility functions.
• Fix commit Window configurations stack management:
  • Update resize-window--window-trim docstring.
  • Make resize-window--apply-config frame aware.
• Insert commit Clear minibuffer on quit.

When I have two frames open, both with split window configurations, I get window changes and buffers appear even when I'm not doing any stack options. I can recreate this just by cycling through windows with w and by invoking ivy-switch-buffer.

Got it! @dpsutton could you please try the current PR instantiation?

I understand window config spans across frames and I am fine with that.

I have a doubt. Shouldn't current-window-configuration get the configuration of a specific frame and current-frame-configuration the configuration of all frames? Or are you talking about the fact that the stack collects the window configuration of all frames?

The problem i am seeing is that I am having buffers appear and disappear just from cycling through windows or changing buffers in another frame.

How do you use ivy-switch-buffer? Do you see any interference with resize-window or lack of integration? Do you switch buffer while resize-window is active?

Thanks!

[dpsutton]

How do you use ivy-switch-buffer? Do you see any interference with resize-window or lack of integration? Do you switch buffer while resize-window is active?

(resize-window-add-choice ?l #'ivy-switch-buffer "switch buffers with ivy") so that i can use it with resize-window and therefore it is active.

I grabbed your patch a second ago and I can't seem to recreate the bug now so that is good :)

About that confusing stuff, I just meant that i understand there is one stack sharing window layouts from multiple frames. I'm used to that. In the past I never kept track of all window configs, just when they were killed down to one or when a manual "mark" was created. I'm not sure I could keep it all in my head. It was a convenience and we might be nearing bookmarks territory :)

[mehw]

Forced push:

• Fix commit Stack forward/backward movements
  • Suspend the notification if the minibuffer is active (ivy-switch-buffer related).
• Insert commit Allow to forcibly replace a key binding.
• Fix commit Formalize docstrings:
  • Update resize-window-add-choice docstring.

@dpsutton I'm addressing the overlay put over the buffer switched by ivy-switch-buffer -- it happens when the configuration is saved, i.e. pressing s, but it seems to be only when switching buffer with ivy, doing that differently doesn't incur into this problem --, and the nuisance of triggering the modification flag when switching buffer.

[mehw]

Forced push:

• Fix commit Display the help menu in a side window:
  • Workaround to force update the current buffer values (ivy-switch-buffer related).
• Fix commit Overlay other windows:
  • Use selected-window rather than get-buffer-window.
• Fix commit Do not resize the root window:
  • Use selected-window rather than get-buffer-window.
• Fix commit Do not delete the main window:
  • Use selected-window rather than get-buffer-window.

@dpsutton I opened an issue about ivy-switch-buffer.

In brief... Soon after ivy-switch-buffer, calling current-buffer or get-buffer-window still references the old buffer, not the one switched to. current-window-configuration captures this odd state and resize-window--add-backgrounds cannot skip the window of the buffer switched to, since it references the old buffer's window... so, an overlay may erroneously be applied to the buffer switched to when it doesn't need to or vice versa.

To overcome this odd behaviour it is preferable to use selected-window rather than get-buffer-window to get the real selected window for sure.

I also implemented a workaround to force update the current buffer values before current-window-configuration captures the configuration...

(resize-window-add-choice ?l #'ivy-switch-buffer "switch buffers with ivy")

With more than one frame open, scrolling the stack to a configuration for another frame and pressing l will expand the minibuffer in that frame. To resume control you need to select that frame... I'm working on this...

[mehw]

@dpsutton Please, take a look at the newly implemented frame awareness feature... I'm currently working on tests and frame related behaviour...

Forced push:

• New commit Frame aware stack management:
• Fix commit Drop current configuration from the stack:
  • Make resize-window--window-drop to use stack utility functions.
• Fix commit Stack forward/backward movements:
  • Fix docstrings.
  • More stack utility functions.
  • Make productive use of resize-window--stack-config-modified.
  • Implement a stack size argument in resize-window--window-trim.
  • Make resize-window--window-modified to use stack utility functions.
  • Fix resize-window--push-stack-head to return the element argument.
  • Fix resize-window--push-stack-tail to return the element argument.
  • Let resize-window--set-stack-head to push a new element if the stack is empty.
  • Let resize-window--set-stack-tail to push a new element if the stack is empty.
  • Let resize-window--set-stack-nth to push a new element if the stack is empty.
• Fix commit Window configurations stack management:
  • In resize-window--refresh-stack rename stack to stack-buffer.
• Fix commit Display the help menu in a side window:
  • Add url of ivy-switch-buffer's issue #1766.

TODO:

• ivy-switch-buffer's switching buffer action (or any other buffer/window related action) should be detected and handled appropriately by resize-window.