diff, show (and others) need improved understanding of commits vs paths

[olsen232]

Git can understand all of the following forms:
git diff PATH PATH
git diff COMMIT PATH PATH
git diff COMMIT COMMIT PATH PATH
git diff COMMIT...COMMIT PATH PATH
git diff -- PATH PATH
git diff COMMIT -- PATH PATH
git diff COMMIT COMMIT -- PATH PATH
git diff COMMIT...COMMIT -- PATH PATH

log, show, and commit have similar (but not identical) patterns.

Since most words eg "foo" could potentially be a reference to a commit, or a path, many of these forms are ambiguous - does the user mean git diff COMMIT COMMIT or git diff PATH PATH?
Git can tell them apart using the -- marker if it is present, or failing that, checking to see if "foo" is a reference to a commit. If "foo" is both a reference to a commit and a path, it warns the user.

Kart does not have this logic, for the most part. kart log is the exception - it works like Git - and kart diff COMMIT PATH PATH at least checks if the first PATH should actually be promoted to COMMIT - but in general, Kart commands don't understand the -- at all, and in general Kart commands assume that the first argument, if present, is a commit, and all subsequent commands, if present, are paths.

The logic for distinguishing commits and paths should be extracted from kart log and generalised for use for all of these commands

[olsen232]

I reused the kart log logic for show and diff as described above, and it works great as long as you only do sensible things. However, I realised much later that it falls apart at the edges, mostly by doing unhelpful operations rather than showing helpful error messages.

The major issue is that we don't check which paths exist when parsing filter arguments. Git does this and so the equivalent Git commands don't do things unless they are sure what you mean: git log A B C D will check if A, B, C and D are commits to be logged, or paths to filter the log by. If they are both / neither, it will error and ask you to be more specific.

Since Kart doesn't do this, if you say kart log A B C D it will assume any of A B C D that it doesn't recognize as a ref / commit, must be a filter. If it was supposed to be a commit but was mistyped, then it will become a useless filter, filtering the results down to nothing.

This bug is worse lately due to spreading to show and diff, where this behaviour is even more surprising - eg kart show A where A is a mistyped commit will instead show HEAD but filtered down to nothing.

We have a lot of options for how to fix this:

1. Go back to previous behaviour (or something like it) where we require a certain amount of REVISION args before we are willing to parse any FILTER args (unless the user forces us to using --)
2. Even stricter + less ambiguous than previous behaviour - assume everything before -- is a REVISION and everything after is a FILTER. Update error messages to prompt users to move things around if they put them in the wrong place.
3. Work harder at guessing which things are intended as revisions. Right now 65fc97511fbb58e5a0e7dd98dc4ebcda56e98cb0 might be a revision, but 65fc97511fbb58e5a0e7dd98dc4ebcda56e98cb1 is not, so it would be treated as a filter. We could guess that the user intends it to be a revision and treat it as such (ie, by returning an error) unless they force it to be a filter using --.
4. Do what Git does and don't just assume non-revisions are filters - only allow them to be filters if we can find a path associated with them. Git's behaviour in this case is somehow to quickly check for the existence of all paths in all commits, I'm not sure how it does this but it would be good if we could leverage it. In the case an argument is both a commit and a filter - or neither - require the user to specify a --

[craigds]

4. ... Git's behaviour in this case is somehow to quickly check for the existence of all paths in all commits

simpler: I think it actually only looks at whether the path exists at HEAD:

# this file was deleted before HEAD
$ git log HEAD kart/exec.py
fatal: ambiguous argument 'kart/exec.py': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

# The file existed at 4644fa6ce257d000496199eec08e6f39e5d2371a, but this still fails because the file didn't exist at HEAD
$ git log 4644fa6ce257d000496199eec08e6f39e5d2371a kart/exec.py
fatal: ambiguous argument 'kart/exec.py': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

ie if you want to get log of a file that's already deleted, I think you have to use --

So, we could do what Git does I think (option 4, but corrected to only look at the paths available in HEAD).

i.e. we could attempt to parse each argument as both a revision and a filter. Parsing filters might mean looking up via dataset schemas available at HEAD. Then we can throw errors for any of:

• some arg isn't a revision AND also isn't a path
• some arg is BOTH a filter and a path
• a path is specified before a revision

Which would match the git behaviour AFAIK