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

Add frame global perspective. #178

Merged
merged 5 commits into from
Apr 12, 2022
Merged

Add frame global perspective. #178

merged 5 commits into from
Apr 12, 2022

Conversation

NicholasBHubbard
Copy link
Contributor

This PR implements a frame global perspective. Any buffer that is a member of the frame global perspective is available for selection when using a perspective aware buffer switcher. Every frame gets its own frame global perspective.

I have been using something like this in my config for a while. It is useful to me because I like to have shell-mode buffers open named *SHELL-{1,5}* that I always want to be able to switch to regardless of what perspective I am in

The way I implemented this is by modifying persp-current-buffers so that it takes an optional arg include-global that if non-nil includes buffers that are members of the perspective named persp-frame-global-perspective-name. Naturally this requires that I also had add the same optional arg to persp-current-buffer-names, persp-is-current-buffer, persp-buffer-filter, and persp-buffer-list-filter. I then modified all of the buffer switchers to use this new option.

I also added wrappers, persp-add-frame-global-buffer, persp-set-frame-global-buffer, persp-remove-frame-global-perspective, and persp-forget-frame-global-perspective. These all work like their generic counterparts but for the frame global perspective.

The name for the frame global perspective is set by the variable persp-frame-global-perspective-name, which defaults to "GLOBAL".

@NicholasBHubbard
Copy link
Contributor Author

Hmm failed the tests for emacs 24.4. I'll have to figure that out.

@NicholasBHubbard
Copy link
Contributor Author

Fixed it :)

Turns out the problem was that persp-current-buffers is also used as a place in setf expressions. So I had to slightly rework the implementation.

I also realized I forgot to modify the ido buffer switcher so I got that to work as well.

@gcv
Copy link
Collaborator

gcv commented Apr 7, 2022

Interesting. I see the value in this feature.

  1. Have you checked for compatibility with persp-state-save and persp-state-load?
  2. After playing with it, I noticed that killing the last buffer associated with the global perspective still leaves the GLOBAL perspective hanging around in the perspective list. It would be nice if it went away when it was no longer needed. That could be implemented using a kill-buffer-hook, though I'm worried that checking every single kill-buffer invocation will be too slow (like the problems in persp-maybe-kill-buffer is too slow #168). Maybe there's a better, clever way to deal with that?
  3. Should something be added to perspective-map? Like g for persp-add-frame-global-buffer?

@NicholasBHubbard
Copy link
Contributor Author

  1. There shouldn't be any problem with persp-state-save and persp-state-load as the GLOBAL perspective is just another perspective in the eyes of the state loading mechanisms. In fact the reason I chose to implement this using a "special" perspective instead of with a global variable that holds a list buffers is for the automatic state saving and loading.

  2. If you think that it is annoying to have the GLOBAL perspective stick around after killing buffers then it probably also annoys you that the GLOBAL perspective is created automatically if it doesn't exist when you call any buffer switcher, even without any explicit calls to persp-add-frame-global-buffer. This behavior hadn't bothered me so I didn't guard against it but I will try to come up with something.

  3. I will add a keybinding.

@NicholasBHubbard
Copy link
Contributor Author

NicholasBHubbard commented Apr 9, 2022

I added a keybinding for persp-add-frame-global-buffer and prevented the GLOBAL perspective from being created implicitly.

As for your request to automatically delete the GLOBAL perspective if it is empty, here are some ideas that all have their own problems:

  1. Adding a kill-buffer-hook like you suggested will not work because of the possibility of a buffer being a member of both the GLOBAL perspective and some other perspective.

  2. I could modify persp-switch so that it checks if the GLOBAL perspective is empty and if so kills it. Then when the user calls persp-switch or any buffer switcher (they all call persp-switch at some level of their call stack because of the modifications I made to make them check the GLOBAL perspectives buffers), the GLOBAL perspective will be deleted. The problem with this is that the GLOBAL perspective will not immediately be killed after it's last buffer is removed. I also think it is just strange to add this behavior to persp-switch as switching perspectives should have nothing to do with killing a perspective.

  3. Make a new function persp-remove-frame-global-buffer that is not defined in terms of persp-remove-buffer so it can implement correct logic for deleting the GLOBAL perspective if it is empty. Then document to the user that the only correct way to remove GLOBAL buffers is with this function. I can only see this causing problems for the user.

Would you merge this PR even if the GLOBAL perspective is not deleted automatically? I think it makes more sense to make it clear in the documentation that the GLOBAL perspective is just another perspective and behaves the same as any other. If this is not acceptable then I think the whole PR should be reimplemented by using a frame-parameter instead of a "special" perspective.

@gcv gcv merged commit 8e1ff5b into nex3:master Apr 12, 2022
@gcv
Copy link
Collaborator

gcv commented Apr 12, 2022

Upon reflection, I'm fine with the global perspective not being deleted automatically. It's an opt-in feature, so users who want it can wrangle with it. (It can be automatically cleaned up with a sufficiently sophisticated kill-buffer-hook, but the needed checks are complicated and potentially slow because that hook runs very frequently.)

I added a bit more documentation, and renamed the main entry point function to persp-add-buffer-to-frame-global, which sounds to me a bit less cryptic. I also had to move some code around to make the byte compiler not issue macro order warnings.

Thank you for contributing to Perspective!

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

Successfully merging this pull request may close these issues.

2 participants