- Notifications
You must be signed in to change notification settings - Fork868
feat(worktree): support adding ignored files with --force#1690
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
feat(worktree): support adding ignored files with --force#1690
Conversation
This adds support for (force add) to include ignored filesin the worktree index. Also improves AddWithOptions and related teststo cover ignored file handling and force scenarios.
onee-only left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hello@MaestroLockedIn! I'm not the maintainer of the project, but I was also interested in the issue. I've left some suggestions. Please have a look and share your thoughts.
| func (w*Worktree)Add(pathstring) (plumbing.Hash,error) { | ||
| // TODO(mcuadros): deprecate in favor of AddWithOption in v6. | ||
| returnw.doAdd(path,make([]gitignore.Pattern,0),false) | ||
| // TODO(mcuadros): deprecate in favor of AddWithOption in v7. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think we don't have to delay this to v7. Currently v6 isn't released and API changes can be made. See#910.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think we don't have to delay this to v7.
I'd concur with this, ideally we would tackle it as part of v6. I'm planning to raise an RFC on the subject of API consolidation which would affect this (and the general "WithOption" funcs), just haven't got around to it yet. In the mean time, we could leave the comment unchanged:
| // TODO(mcuadros): deprecate in favor of AddWithOption inv7. | |
| // TODO(mcuadros): deprecate in favor of AddWithOption inv6. |
| dirPatterns,_:=gitignore.ReadPatterns(w.Filesystem,nil) | ||
| patterns=append(patterns,dirPatterns...) | ||
| }else { | ||
| dirPatterns,_:=gitignore.ReadPatterns(w.Filesystem, []string{dir}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As far as I know,gitignore.ReadPatterns reads the directories recursively starting from the specified directory. So I think the case below won't work as expected:
- root directory contains
.gitignorewith content:foo. - user adds
foo/bar.txt
In this case, since it isn't aware of the root's.gitignore, it will addfoo/bar.txt.
Could you please test the code against this case?
Issue: Add() should not add ignored files by default#1679
This PR adds support for force-adding ignored files using the --force option, mirroring standard Git’s behavior (git add -f).
Changes
Updated AddWithOptions():
Supports Force: true to bypass .gitignore patterns.
Modified doAdd() and doAddDirectory() to correctly include ignored files when forced.
Added comprehensive tests:
TestAddWithForceIgnoredFile
Minor cleanup and improved .gitignore pattern handling.