Add check-ignore as command that allows placeholders to be created #362
Conversation
| GitCommandLineParser.Verbs.AddOrStage | | ||
| GitCommandLineParser.Verbs.Move | | ||
| GitCommandLineParser.Verbs.Status | | ||
| GitCommandLineParser.Verbs.CheckIgnore; |
There was a problem hiding this comment.
Do we always want to allow check-ignore, or only when it has a --stdin parameter? What about other verbs that take a --stdin parameter, could they be in a similar boat?
There was a problem hiding this comment.
@wilbaker would like to at some point completely remove this check and allow all commands to create placeholders but there are a bunch of tests that fail, mainly conflict and reset ones I believe. I don't think we need to check for --stdin since this command is only checking if a path is ignored or excluded.
Yes other commands would be in this same boat if a third party app started the command and kept it around while trying to do read files. I will see what all the commands are that fall into this category.
There was a problem hiding this comment.
Commands that could be waiting on stdin
check-attr
check-ignore
check-mailmap
checkout-index
commit-graph
diff-tree
fetch-pack
hash-object
index-pack
name-rev
notes
reset - Experimental
send-pack
rev-list
update-index
update-ref
The fact that these wait on stdin isn't necessarily an issue, but placeholders will not be able to be created if these are running and waiting on input.
There was a problem hiding this comment.
Running some tests and the failures are for:
check-ignore
check-mailmap
checkout-index
reset
update-index
Most of the others are in the list to not get the lock which is probably where check-ignore and check-mailmap belong. I'll update the PR. The others will have to stay for now since they are changing the index.
| GitCommandLineParser.Verbs.AddOrStage | | ||
| GitCommandLineParser.Verbs.Move | | ||
| GitCommandLineParser.Verbs.Status | | ||
| GitCommandLineParser.Verbs.CheckIgnore; |
There was a problem hiding this comment.
Can we add a functional test that fails without this change?
There was a problem hiding this comment.
Yep I was looking into this and we have one for AddOrStage that we can copy from.
kewillford
force-pushed
the
check_ignore
branch
from
c44e72e
to
51e73dc
Compare
| [TestCase("send-pack")] | ||
| [TestCase("rev-list")] | ||
| [TestCase("update-ref")] | ||
| public void AllowsPlaceholderCreation(string commandToRun) |
There was a problem hiding this comment.
Nit: AllowsPlaceholderCreationWhileGitCommandIsRunning?
kewillford
force-pushed
the
check_ignore
branch
from
51e73dc
to
c9ee2f7
Compare
kewillford
force-pushed
the
check_ignore
branch
from
c9ee2f7
to
34a54f9
Compare
kewillford
force-pushed
the
check_ignore
branch
from
e0fec6d
to
23d457d
Compare
This fixes #361