-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: add option to exclude files globally #5109
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
base: master
Are you sure you want to change the base?
Conversation
|
I used Claude Code extensively to help me implement this--hope you don't mind. I did have it iterate a few times where it wasn't doing things in a sensible way and to make it follow the conventions of the code base. |
The action for ignoring a file says "Ignore or exclude file", but the action for excluding a file says "Exclude file"; this asymmetry doesn't make sense. We should either add a more specific "Ignore file" string for ignoring a file, or use the more generic one for excluding too. I decided to do the latter, since it's simpler, and I find it good enough.
Now that all callers pass the same one, we can hard-code it in the function.
Add a third option to the ignore/exclude menu that writes to the user's global git excludes file. This complements the existing options: - Add to .gitignore (shared, this repo) - Add to .git/info/exclude (private, this repo) - Add to ~/.config/git/ignore (private, all repos) <- NEW The new option respects core.excludesFile if configured, otherwise falls back to the XDG-compliant default location ($XDG_CONFIG_HOME/git/ignore or ~/.config/git/ignore). The menu displays the actual file path that will be modified, making the behavior predictable and transparent. Closes jesseduffield#5108
Return error if we can't resolve the user's home directory. Without this it would create a file with the literal name `~/...` in the current directory, which is never what we want.
Use fmt.Sprintf instead of template syntax to substitute the path name. For texts with only a single placeholder this is good enough, and it's a bit simpler.
Was showing the wrong error when trying to ignore the .gitignore file.
Add test coverage.
7006600 to
76d994c
Compare
|
Thanks, good work. It still took some more work to get it to a state that I'm happy with:
I'm happy with this in the current state, but please double-check my changes. |
|
Thanks for taking a look and making these fixes. All seems good to me. The help text is nice, though if core.excludesFile is configured specially for the repo you're in, it won't actually be shared for all repos. But I don't expect this is a common configuration. This, along with not wanting to clutter the interface, was why I didn't try to add clarifying text myself. |
Right, I forgot that core.excludesFile could also be a local config. I agree that's probably uncommon, but if we wanted to get fancy we could actually work out whether it's local or global (using |
Summary
Add a third option to the ignore/exclude menu that writes to the user's global git excludes file. This complements the existing options:
.gitignore.git/info/excludecore.excludesFileor XDG defaultThe new option:
core.excludesFileif configured$XDG_CONFIG_HOME/git/ignoreor~/.config/git/ignore)Screenshot
The menu now shows three options, with the global option displaying the resolved path:
Test plan
core.excludesFileis respected when configuredCloses #5108