Don't block checking out a PR if there are untracked files #1302
Conversation
Working directory should appear clean when there are untracked files. `UntrackedFile_IsWorkingDirectoryClean_True` is currently failing.
We don't allow checkout when: A file has been added to index A file has been modified in working dir A file has been staged A file is missing from working dir A file has been removed from index A file has been renamed in index A file has been renamed in working dir We do allow checkout when: The index and working dir is unaltered since last commit There are untracked files
These might be build or tool artefacts that shouldn't prevent a PR from being checked out.
| return Observable.Return(isClean); | ||
| } | ||
|
|
||
| static bool IsFilthy(RepositoryStatus status) |
There was a problem hiding this comment.
Is this actually needed though? Can't we just let libgit2sharp error out if it doesn't manage the operation? I honestly can't remember why I felt it necessary to do a check for cleanliness here :/
Also LOL at the name ;)
There was a problem hiding this comment.
Is this actually needed though? Can't we just let libgit2sharp error out if it doesn't manage the operation?
I wanted to start with a simple drop-in replacement for what we've been using (a simple bug fix). Letting libgit2sharp error out might be better and give the user more info, but would also require more testing.
I honestly can't remember why I felt it necessary to do a check for cleanliness here :/
I know command line Git lets you checkout when there modified files, as long as they don't conflict with changes in the branch you're checking out. I'm not sure this is something we would want to allow in this context. Maybe this is an option, I'll investigate.
If you're happy with this simple fix, I'd prefer to start with that and improve later. Can I open a separate PR that uses the letting libgit2sharp error out technique?
There was a problem hiding this comment.
Here is a PR that lets libgit2sharp error out rather than simply allowing un-tracked files: #1304. I'm going to document the implications of this further and get more eyes on it.
Until #1304 is approved, I'm keen to get this PR merged as a point fix. I don't mind reverting it later, but I'm sick of not being able to check-out PRs! 😉
Does that make sense?
| Commands.Stage(repo, path); | ||
| repo.Commit("foo", Author, Author); | ||
| File.Move(file, renamedFile); | ||
| // NOTE: Renamed files appear as Missing and Untracked rather than RenamedInWorkingDir. Is this a bug? |
There was a problem hiding this comment.
@ethomson I'm surprised that renaming a (committed) file didn't make it appear as RenamedInWorkingDir. It instead appears as Missing and Untracked. This also makes sense, but I wonder what RenamedInWorkingDir is actually for? Might this be a bug or am I missing something obvious?
There was a problem hiding this comment.
You need to query for working directory renames explicitly in your Status, they're not included by default. Since they look at every untracked file, they're particularly expensive to calculate and I wouldn't recommend it, tbqh.
We added it to libgit2/LibGit2Sharp for Visual Studio 2013's Team Explorer when working in the mode where it hid the stage. (If you have no staging area, you can't calculate renames between the HEAD and the index, so it calculated unstaged renames in the working directory).
Note that this default (not detecting renames in the working directory) matches the behavior of https://github.com/git/git where status only shows staged renames (and in fact has no option to enable unstaged renames in the working directory).
There was a problem hiding this comment.
Thanks for clearing up my confusion!
By default, renamed files appear as Missing and Untracked rather than RenamedInWorkingDir.
Loosen slightly when a PR can be checked out.
We don't allow checkout when:
We do allow checkout when:
This isn't quite a loose as when checking out from the command line, but it looser than is was before (when only Ignored and Unaltered files were alowed).
Fixes #1271