Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #1281 - window-purpose layer #1958

Closed
wants to merge 1 commit into from

Conversation

bmag
Copy link
Collaborator

@bmag bmag commented Jun 11, 2015

After using a private layer for two months, I've started working on a proper contrib layer for window-purpose. It is still a work-in-progress, and I'd like to hear any suggestions or questions you may have. Note that this PR doesn't try to replace anything currently in Spacemacs, just adds an optional layer that improves (IMHO) some parts of the window manager.

Here is what I've done or planning to do in this PR:

  • enable window-purpose package and define some key bindings
  • extend window-purpose to give the same behavior as popwin, so there's (almost?) no regression in behavior for users. See extension purpose-popwin.el.
  • make window-purpose and popwin play together nicely without conflicts
  • empty purpose-mode-map, because by default it overrides C-x C-f, C-x b, etc. and conflicts with semantic over C-c , key bindings.
  • create a helm-mini-like helm command to use instead of purpose-switch-buffer-with-purpose and purpose-switch-buffer-with-some-purpose

Especially I'd appreciate advice about:

  • how to write the helm commands - I've had some bugs when I tried, maybe someone else can give it a try?
  • key bindings: I've used the SPC r prefix ("r" for "puRpose"), since it's relatively short and doesn't have many other bindings. Do you prefer I use different keys?
  • extra purposes to configure for Spacemacs? I think the default purposes will be fine for now.
  • window-purpose has commands for saving/loading a window layout to/from file. I'm not sure what other methods Spacemacs has for saving/loading a window layout, so I didn't do anything about it yet.

Some explanation about window-purpose: (no need to read if you're familiar with the package)

Basically, window-purpose is an alternative to the behavior of stock display-buffer and switch-to-buffer. When stock Emacs chooses a window for a buffer, it can either:

  • reuse a window that already shows that buffer
  • use some window that shows another buffer
  • create a new window/frame

The problem is that with "use some window" it's hard to predict where the window will open. window-purpose adds another option: "reuse a window that shows another buffer that has the same purpose as the buffer that needs to be displayed." This makes the behavior more predictable and makes the window layout more robust.

Another thing that window-purpose allows is to dedicate a window to a purpose (rather than a single buffer). So if, for example, I dedicate a window to the purpose edit (default purpose for code files), Emacs will use it only for code files and I can safely think of it as my code window. Similarly I can have a REPL
window or any other type of window I may want - it's all customizable.

What's more, there's an equivalent for display-buffer-alist, so we can define special behaviors for different kinds of windows. The first thing that comes to mind is that we can define a single consistent language-agnostic behavior for all the different REPLs in Spacemacs.

Another short-coming of stock Emacs, IMHO, is that switch-to-buffer doesn't follow the rules in display-buffer. window-purpose enforces its rules also on switch-to-buffer. Of course there are still differences between switch-to-buffer, switch-to-buffer-other-window, switch-to-buffer-other-frame, pop-to-buffer, pop-to-buffer-same-window and display-buffer, but now they all use the same mechanism.

@rphillips
Copy link

Nice work... Do you know if this plays nice with eyebrowse?

@bmag
Copy link
Collaborator Author

bmag commented Jun 11, 2015

I haven't started using eyebrowse yet, so I don't know. I hope it does. If it doesn't, I'll have to figure out something.

EDIT: a short test reveals that for the purpose-dedication state to be saved and restored when switching workspaces, you need to run this code:

(setq window-persistent-parameters (append '((purpose-dedicated . writable)) window-persistent-parameters))

That code should probably be added upstream to window-purpose.

@bmag bmag changed the title [WIP] window-purpose layer window-purpose layer Jun 12, 2015
@bmag
Copy link
Collaborator Author

bmag commented Jun 12, 2015

This PR is ready to merge. Any questions, suggestions, objections, etc?

@rphillips it should play nice with eyebrowse now.

@tuhdo
Copy link
Contributor

tuhdo commented Jun 12, 2015

I'm really excited to try this out.

@bmag
Copy link
Collaborator Author

bmag commented Jun 13, 2015

@tuhdo glad to hear that.

Found and fixed a minor issue with eyebrowse. Basically this layer is ready and usable, but it is likely some (currently unknown) packages will require some tweaking to work with it. I'm prepared to fix any such issues we might find.

@syl20bnr
Copy link
Owner

Added to milestone 0.105

@CestDiego
Copy link
Contributor

I hope this works with perspectives layer as well x3

@robbyoconnor
Copy link
Contributor

Any chance of rebasing this?

@bmag
Copy link
Collaborator Author

bmag commented Sep 17, 2015

@CestDiego Most of what it does is to change the behavior of display-buffer and switch-to-buffer, so that shouldn't be a problem. There are a few purpose-related commands for switching buffers, which I think offer buffers from all perspectives as candidates. IIRC there is a with-perspective-buffers (or similar) macro that could be used to wrap those commands.
Other than that, window-purpose can save and restore layouts. It doesn't save the actual buffers, but it remembers the purpose of each window. When loading a layout, it looks for a suitable buffer to show in each window. I think that also here the buffers won't be limited to the current perspective, and it needs to be wrapped with the same macro.
There may be some issues with the window layout when switching perspectives, depending on the implementation in persp-mode. If it's done the same way as eyebrowse (IOW by using window-state-put) then it should be fine.
There could be more, but I'm pretty sure that's all.

@robbyoconnor Will rebase it later today or tomorrow, but right now I don't have time

@bmag bmag force-pushed the window-purpose-2 branch 2 times, most recently from dc58e4f to 2c00d24 Compare September 18, 2015 10:41
@bmag
Copy link
Collaborator Author

bmag commented Sep 18, 2015

rebased and squashed

@robbyoconnor
Copy link
Contributor

👍 ❤️

@fatlazycat
Copy link

@bmag hi, tried your PR as a private layer but failed to pass the tests when loading. Should it be fine ?

@bmag
Copy link
Collaborator Author

bmag commented Dec 2, 2015

@fatlazycat what tests are you referring to? I am also using it as a private layer, without a problem. Perhaps it's an installation issue?

@fatlazycat
Copy link

@bmag odd must have been PEBKAC, tried it again and loaded fine. Thanks.

@tuhdo
Copy link
Contributor

tuhdo commented Dec 3, 2015

@bmag You should integrate window-purpose with helm-find-files. Current purpose-find-file has no action menu for i.e. searching with C-s.

@robbyoconnor
Copy link
Contributor

👍
On 12/02/2015 10:24 PM, Tu Do wrote:

@bmag https://github.com/bmag You should integrate |window-purpose|
with |helm-find-files|. Current |purpose-find-file| has no action menu
for i.e. searching with |C-s|.


Reply to this email directly or view it on GitHub
#1958 (comment).

@bmag
Copy link
Collaborator Author

bmag commented Dec 3, 2015

@tuhdo I think you meant purpose-switch-buffer, in which case I agree. I plan to write some helm sources for the window-purpose package, but it's a low priority for me and I'm very busy with non-Emacs related stuff these days, so it's not coming soon, sorry.
On the good side, this layer (not the package) does add a helm source that inherits from helm-buffers-list, and binds it to SPC r b, so that one should have the same action menu, including C-s.

@bmag
Copy link
Collaborator Author

bmag commented Mar 3, 2016

updated and rebased

@syl20bnr syl20bnr changed the title window-purpose layer Fixes #1281 - window-purpose layer Mar 5, 2016
@a13ph
Copy link

a13ph commented Mar 18, 2016

This is very very nice.
And it plays well with my ideas about persistent help window (think discoverability to the max and constant reminder/cheatsheet).
So i'll definitely play around with it at some point and try to think of some useful purposes for it

@onemanstartup
Copy link

I want to use this for tdd. Right now I'm learning emacs and run ert tests and it opens new buffer and I need to type 'q' to quit because it switches focus. Does anyone know how to run command and NOT switch buffers? Or some solution to stay in buffer where I'm writing code?

@nixmaniack
Copy link
Contributor

@onemanstartup You can use popwin config for this.

(push '("*YourBufferName*"          :dedicated t :position bottom :stick t :noselect t   :height 0.4) popwin:special-display-config)

Replace *YourBufferName* accordingly. This would popup buffer at bottom and won't be selected by default. You can use C-g or SPC w p p to close it.

@d12frosted
Copy link
Collaborator

This is awesome! Thanks for sharing!

@trishume
Copy link
Contributor

This is a really great addition and will really improve the user experience of Spacemacs. I actually might even suggest that this be added by default in new .spacemacs files.

When I switched back to using Spacemacs for work recently the window opening behavior constantly clobbering buffers is a constant annoyance. I remembered talk of window-purpose from back when I first used Spacemacs and thought "that must be included by now" but alas it still was not.

Although this may in fact be an effective strategy for getting me to contribute to Spacemacs again. Since I have to use Spacemacs for work, eventually I may get so annoyed at the window opening behavior that if someone tells me what is needed to get this merged, I'll do it myself some weekend.

@nixmaniack
Copy link
Contributor

I've been using this layer since past 6+ months and without this I find window management out of place. It would be very important addition if we can get this in upcoming 0.200 release.

@CestDiego
Copy link
Contributor

<3

On Tue, Sep 20, 2016, 2:30 PM Muneeb Shaikh notifications@github.com
wrote:

I've been using this layer since past 6+ months and without this I find
window management out of place. It would be very important addition if we
can get this in upcoming 0.200 release.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1958 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADI546EjDTZvr94yJHV5fd5A_Xi7jHOCks5qsCY0gaJpZM4FAITT
.

Diego Berrocal
http://cestdiego.github.io

@syl20bnr
Copy link
Owner

This is planned for the next release. It was originally planned for something like 0.104 but each time there were additional stuff stepping in so I had to delay it.

@syl20bnr
Copy link
Owner

@trishume I don't think there is much to do to have it merged except waiting for the 0.200 release, moreover @bmag is the author of this package so we have already the best person to handle this ;-)

@syl20bnr
Copy link
Owner

@trishume welcome back by the way :-D

- Proper integration with popwin
- Integrate with Helm via helm-purpose
- Integrate with Ivy via ivy-purpose
- Integrate with opening a new eyebrowse workspace
- Enable purpose-x-kill: purpose-aware replacement of a window's buffer
  when a  buffer is killed
@bmag
Copy link
Collaborator Author

bmag commented Sep 21, 2016

Updated the PR with ivy-purpose support and rebased. FYI the layer in this PR is a copy of the layer from my config (see here), and from time to time I update this PR.

@d12frosted
Copy link
Collaborator

@bmag this is so lovely. My favourite and long waited PR. Thank you for window-purpose and this layer (lawyer) 😸 💃

@trishume
Copy link
Contributor

I've been using this layer for a day and I may have found a fairly annoying bug. It seems that the paste transient state confuses window purpose some but not all of the time and replaces the contents of the buffer you are pasting in with the hydra docstring. Yes, the buffer, you have to use u to get the contents back. It also tries to kill your buffer when it exits the hydra and you but the prompt to save the changed contents allows you to stop it.

@bmag
Copy link
Collaborator Author

bmag commented Sep 27, 2016

@trishume I've seen something like this about 1-2 weeks ago, but wasn't sure if it's because of window-purpose or not, and couldn't find out a pattern. Can you reproduce the problem reliably? Does the problem go away if you run M-x purpose-x-kill-unset?

@trishume
Copy link
Contributor

@bmag I'll try and figure out how to reproduce it reliably and let you know if I do. After I restart my emacs because it's completely hung, as happens near-daily. My return to Spacemacs/emacs has not been going smoothly so far...

@bmag
Copy link
Collaborator Author

bmag commented Sep 27, 2016

@trishume I figured out how to trigger the bug, I should have a fix today or tomorrow

@trishume
Copy link
Contributor

@bmag cool. Meanwhile it's started happening to me every time making it really difficult to paste, and purpose-x-kill-unset doesn't help, so I'm gonna disable window-purpose until then.

@bmag
Copy link
Collaborator Author

bmag commented Sep 27, 2016

@trishume turns out Hydra's buffer name changed two weeks ago, so I had to update it in window-purpose as well (the name is hard-coded in Hydra). Fixed it, new version will be available from MELPA in a few hours.

@syl20bnr
Copy link
Owner

One of the oldest PR :-D
It is now in develop !
I renamed the layer to spacemacs-purpose and it is added automatically with the spacemacs distribution.
I made a couple a fixes in separate commits, please check the history to see if I did mistake.

Thank you ! 💯 💜
Cherry-picked into develop branch, you can safely delete your branch.

@d12frosted
Copy link
Collaborator

Woooo... it's in! 🎉 🎉 🎉

@bmag
Copy link
Collaborator Author

bmag commented Oct 17, 2016

I (and also @nixmaniack) found only one error, and @TheBB already fixed it with #7441. I'll go over my personal config and check if there's something worth adding that wasn't in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.