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

projectile-current-project-on-switch 'keep moves current project to front #1879

Closed
wants to merge 1 commit into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Feb 2, 2024

Problem

There are certain modes for which the
projectile-find-file-hook-function function does not run by
default. This includes, shell, eshell, and magit. If one
navigates directly to one of these buffers and calls
projectile-relevant-known-projects while
projectile-current-project-on-switch is set to 'keep they will
notice that the first repository in the list is not the one that they
expect

Solution

There are three possible solutions that come immediately to mind. I
want to mention all three of them for consideration though naturally
this PR only implements one of them:

  1. Have users instrument all modes to call projectile-find-file-hook-function
  2. Modify this code to do what is "expected"
  3. Modify projectile-mode to add the hook to all possible modes

I've opted with Solution 2 because it feels more robust in the long
term, as new modes appear more changes will not be required. It also
provides less of burden on package users.

The principle drawback is that it will not be added to the projects
list in this case so navigating to a different buffer will not save
the position of this project. This issue can be solved by a further
modification where any call to projectile-relevant-known-projects
(or any other function in this call chain) will lead to an update to
the list of projects. I'd be willing to take that on as well.

Solution 3 also feel promising, not sure if we could do something
with eval-after-load


Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
    I've added a new test that was failing before my change and passes now
  • All tests are passing (eldev test)
    All existing tests continue to pass without modification implying that
    no defined behavior was chagned
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)

Thanks!

projectile.el Outdated Show resolved Hide resolved
@bbatsov
Copy link
Owner

bbatsov commented Feb 6, 2024

Doing something in the spirit of super-save might be an option as well https://github.com/bbatsov/super-save/blob/master/super-save.el#L47

Instead of relying on a hook, we can have a list of navigation commands that act as a trigger for the current project to be recomputed. I don't recall why I chose the find-file-hook back in the day - most likely because this was the simplest approach, regardless of its limitations.

@bbatsov
Copy link
Owner

bbatsov commented Feb 17, 2024

@jtamagnan-delphix ping :-)

@ghost
Copy link
Author

ghost commented Feb 17, 2024

@bbatsov, thanks for the reminder and quick feedback!


I updated the code with the 'front solution, and ran the tests. It seems to work planned. I also really like the "list of navigation commands" approach. I'd be willing to take that on in this PR or a following one. I can't make promises on timeline for that though, free time has been less plentiful. I'm happy to have this PR linger as a reminder to finish up this work, up to you.

@ghost
Copy link
Author

ghost commented Apr 23, 2024

@bbatsov I've been thinking about this a little bit more. It seems like a change of "project" could happen from anything, even something as simple as a call to other-window where we change the buffer. With that in mind it actually seems like the only hook that matters and the one that that we'd want to modify is buffer-list-update-hook which seems to be triggered whenever we change the buffer that is selected.

With that in mind it seems like the only "bullet-proof" solution to ensuring that the correct current project is at the tip of the list is to add projectile-track-known-projects-find-file-hook to buffer-list-update-hook or to go with this simple 'front work around that I've been using.

I'm happy to get this change in as is or modify the minor-mode to set the hook on buffer-list-update-hook if you think that makes sense. TIA

@jtamagnan
Copy link
Contributor

@bbatsov due to some GitHub account issues I faced I accidently ended up deleting this pull request. I've however gone ahead and recreated this PR as well as the alternative approach in #1894 and #1895, I consider both approaches mutually exclusive but would tend to lean towards #1895 despite it being Emacs 28.1+ exclusive.

This pull request was closed.
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