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 ibuffer-vc-buffer-file-name-function option #34

Merged
merged 1 commit into from
May 30, 2023
Merged

Add ibuffer-vc-buffer-file-name-function option #34

merged 1 commit into from
May 30, 2023

Conversation

tsilvap
Copy link
Contributor

@tsilvap tsilvap commented May 10, 2023

This is one way of fixing #25. To recap the issue:

Expected behavior

Only file buffers, "directory buffers" (e.g. Dired buffers), and other VC-related buffers (e.g. Magit buffers) should show up in the VC groups. Examples of buffers that should not show up are: ERC, Gnus, *Help*, *info*, or any other buffer at all that is not related to that VC directory.

Actual behavior

If you're visiting a VC-controlled file, and you open an unrelated buffer such as those from ERC, or Gnus, or a *Help* buffer, etc., it'll show up in the VC group (because the buffer's default-directory will point to a VC-controlled directory).

Proposed solution

@c-alpha's suggestion in the issue thread works great but it excludes e.g. Dired and Magit buffers, which IMO should go in the VC filter group too. And other people may have other preferences. But right now I don't think we can customize that since ibuffer-vc-include-functiontakes a filename, instead of a buffer.

If we introduce a new ibuffer-vc-include-buffer-function option that takes a buffer instead, the user can have more control. e.g. I could implement the expected behavior with:

(setq ibuffer-vc-include-buffer-function
      (lambda (buf)
        (with-current-buffer buf
          (or buffer-file-name
              list-buffers-directory)))) ; this is set by Dired buffers, Magit, VC dir, etc.

@purcell
Copy link
Owner

purcell commented May 15, 2023

There's already a chunk of code that determines the file or a given buffer (with (or buffer-file-name default-directory)) — maybe we should just make that customisable via a function? I think it would have the same effect.

@tsilvap
Copy link
Contributor Author

tsilvap commented May 18, 2023

Hey @purcell, thanks for the feedback. Your suggestion makes sense to me, I just force-pushed a commit that implements it. Let me know if I misunderstood what you meant.

@tsilvap
Copy link
Contributor Author

tsilvap commented May 22, 2023

BTW, I think this is a better default for the ibuffer-vc-buffer-file-name-function:

(lambda (buf)
  (with-current-buffer buf
    (file-truename (or buffer-file-name
                       list-buffers-directory))))

In general, only buffers like Dired, Magit, VC dir, etc. ("directory buffers") set the list-buffers-directory buffer-local variable, meaning in each VC group we will only have file buffers of each VC-controlled project, plus directory buffers of each project.

Compared to now, if we open any non-file buffer (*Org Agenda*, *Calendar*, etc.) from a VC-controlled file buffer, that buffer will also show up in the VC group (because default-directory will be set to the directory of that file), even though such buffers have nothing to do with VC.

I think it would be good to make that change. Of course it's a "backward incompatible" change insofar as we'd change the previous behavior of which buffers get grouped, but IMO the upside of having a saner default behavior is worth it.

If you agree @purcell, let me know and I can update this PR or create a new one.

@tsilvap tsilvap changed the title Add ibuffer-vc-include-buffer-function option Add ibuffer-vc-buffer-file-name-function option May 22, 2023
@purcell
Copy link
Owner

purcell commented May 22, 2023

That does sound like a better default, yes. I think we should give that a go.

@tsilvap
Copy link
Contributor Author

tsilvap commented May 28, 2023

Sure thing. I force-pushed a new commit that changes the default behavior to that instead.

Hopefully users will agree that this is a better default and find it more useful, but if the old behavior is preferred, it can be restored by customizing the new ibuffer-vc-buffer-file-name-function variable, like so:

(setq ibuffer-vc-buffer-file-name-function
      (lambda (buf)
        (with-current-buffer buf
          (file-truename (or buffer-file-name
                             default-directory)))))

@purcell purcell merged commit ac07ed3 into purcell:master May 30, 2023
9 checks passed
@purcell
Copy link
Owner

purcell commented May 30, 2023

Merged, thanks!

@purcell
Copy link
Owner

purcell commented Jun 8, 2023

This feels like how things should always have been, thanks for the fix.

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.

None yet

2 participants