fix CLI for single revisions
The CLI unconditionally passes all argument to rev-list, even if the argument is a single commit rather than a revision range. This causes the CLI to always analyse the entire git history up to the given commit, rather than just that commit.
Introduced in d601e35f.
I appears that things like master~3 and mybranch^ still work, because they result in a single commit that is resolved by get_commit in find_dependencies.
I'm not sure about mybranch^!. It is explained (in the git-revisions man page) as specifying a range, but I believe it actually just means the commit at the head of mybranch. That doesn't work anymore (it doesn't work in the GUI either), but then you might as well just write mybranch?
To specify ranges, the current code still handles the .. and ... notations as you would expect.
I think that we want this to behave quite like git show:
- Given a single commit, it acts on just that commit (unlike
git logorgit rev-list, which include the commit plus all commits that are reachable from it). - Given a range, it acts on all commits in the range.
I think that we could do that by first trying detector.get_commit, and if we catch InvalidCommitish, try GitUtils.rev_list.
@robhoes commented on 22 Jan 2018, 11:34 GMT:
I appears that things like
master~3andmybranch^still work, because they result in a single commit that is resolved byget_commitinfind_dependencies.
Correct.
I'm not sure about
mybranch^!. It is explained (in thegit-revisionsman page)
I guess you mean the git-rev-parse man page here?
as specifying a range, but I believe it actually just means the commit at the head of
mybranch.
Yes, that's a range containing a single commit.
That doesn't work anymore (it doesn't work in the GUI either) but then you might as well just write
mybranch?
Ah, I think I see the confusion. I already changed this behaviour in 4caf25ab but that's only the module branch right now, which I haven't yet gotten round to merging into master.
To specify ranges, the current code still handles the
..and...notations as you would expect.I think that we want this to behave quite like
git show:
- Given a single commit, it acts on just that commit (unlike
git logorgit rev-list, which include the commit plus all commits that are reachable from it).- Given a range, it acts on all commits in the range.
Yes, exactly - that's what 4caf25ab does.
I think that we could do that by first trying
detector.get_commit, and if we catchInvalidCommitish, tryGitUtils.rev_list.
It looks like I made the odd decision to fork a git rev-list subprocess to do this, but since it was >=19 months ago, I can't remember why I did that :-/
@aspiers I'm not sure if you had seen my updated patch. It's quite straightforward and I believe that it does the right thing in all cases.
Issue was reported in #82.