git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Suggestion: better error message when an ambiguous checkout is executed
@ 2017-08-07 21:49 Mahmoud Al-Qudsi
  2017-08-07 22:44 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Mahmoud Al-Qudsi @ 2017-08-07 21:49 UTC (permalink / raw)
  To: git

Hello,

The default git behavior when attempting to `git checkout xxx` for
some value of "xxx" that cannot be resolved to a single, unique
file/path/branch/tag/commit/etc is to display the following:

> error: pathspec 'xxx' did not match any file(s) known to git

Unfortunately, this is (IMHO) at best misleading when the actual case
is that "git could not unambiguously resolve pathspec xxx"

Can the case where xxx _was_ resolved but to more than one value be
improved in both utility and comprehensibility by providing an error
message that

1) indicates that xxx was a valid pathspec, but not a unique one
2) provides a list of unique pathspecs that xxx matched against

e.g. in the case where xxx is the name of a branch on both origin1 and
origin2, it would be ideal if git could instead report

> error: pathspec 'xxx' could not be uniquely resolved
> xxx can refer to one of the following:
> * branch origin1/xxx
> * branch origin2/xxx

or, less ideally but much simpler, only the first line of that message?

Thank you,

Mahmoud Al-Qudsi
NeoSmart Technologies

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Suggestion: better error message when an ambiguous checkout is executed
  2017-08-07 21:49 Suggestion: better error message when an ambiguous checkout is executed Mahmoud Al-Qudsi
@ 2017-08-07 22:44 ` Junio C Hamano
  2017-09-12  6:53   ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2017-08-07 22:44 UTC (permalink / raw)
  To: Mahmoud Al-Qudsi; +Cc: git

Mahmoud Al-Qudsi <mqudsi@neosmart.net> writes:

> The default git behavior when attempting to `git checkout xxx` for
> some value of "xxx" that cannot be resolved to a single, unique
> file/path/branch/tag/commit/etc is to display the following:
>
>> error: pathspec 'xxx' did not match any file(s) known to git

Yes, it is true that the user may have wanted to instead checkout a
branch 'xxy' and misspelled it as 'xxx'.  Or the user may have more
than one remotes, from which there are remote-tracking branches for
'xxx' branch.  Neither of these cases allow Git to interpret 'xxx'
as a rev, and Git blindly thinks that 'xxx' must be a pathspec, and
wants to ensure that such a path exists in the working tree (if
'xxx' does not look like a wildcard or otherwise magic pathspec).

> Unfortunately, this is (IMHO) at best misleading when the actual case
> is that "git could not unambiguously resolve pathspec xxx"

The actual case you want to address is "git could not tell that the
user meant 'xxx' as a revision, even though the end user meant it as
such".

> Can the case where xxx _was_ resolved but to more than one value be
> improved in both utility and comprehensibility by providing an error
> message that
>
> 1) indicates that xxx was a valid pathspec, but not a unique one

Just the terminology, you are no longer talking about a pathspec.
You are talking about a rev; i.e. when refs/remotes/origin[12]/xxx 
exist, the user may have meant 'xxx' as a rev, but Git is not allowed
to pick one of them randomly.

It would be nice to take this case into account.

Note that if refs/remotes/origin/xxy exists and the user misspelled
it as 'xxx', you would still get the same "(because 'xxx' cannot be
a rev, it must be a pathspec) pathspec 'xxx' did not match..." error
message, though, so there might not be much point in special casing
"more than one potentially matching revs" case over "there is no
potentially matching revs" case, though.

> 2) provides a list of unique pathspecs that xxx matched against
>
> e.g. in the case where xxx is the name of a branch on both origin1 and
> origin2, it would be ideal if git could instead report
>
>> error: pathspec 'xxx' could not be uniquely resolved
>> xxx can refer to one of the following:
>> * branch origin1/xxx
>> * branch origin2/xxx

Again you are talking about "revs", not pathspecs.  The above (with
tweak to the wrong terminology) would work as a better error message
*if* there is no chance that the user meant 'xxx' as a pathspec,
i.e. "I want to overwrite the files in the working tree that matches
the pathspec 'xxx' with matching contents from the index".

So a possible implementation approach would be

 - to let the current code do what it is doing

 - except that you add new code immediately before the code that
   issues 'xxx' did not match (i.e. the code already checked that
   'xxx' taken as a pathspec does not match anything, and about to
   give the error message but hasn't done so just yet).

 - your new code 

   . checks if 'xxx' could be an attempt to refer to a rev but
     insufficiently spelled out (e.g. both origin[12]/xxx exists, or
     for a bonus point, a similarly named origin/xxy exists and
     could be a typo).

   . if the above check found something, then you report it and
     terminate without complaining "pathspec 'xxx' did not
     match..."

   . on the other hand, if the above check did not find anything,
     then you let the current code issue the same error message as
     before.



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Suggestion: better error message when an ambiguous checkout is executed
  2017-08-07 22:44 ` Junio C Hamano
@ 2017-09-12  6:53   ` Junio C Hamano
  0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2017-09-12  6:53 UTC (permalink / raw)
  To: Mahmoud Al-Qudsi; +Cc: git

Junio C Hamano <gitster@pobox.com> writes:

> Mahmoud Al-Qudsi <mqudsi@neosmart.net> writes:
>
>> The default git behavior when attempting to `git checkout xxx` for
>> some value of "xxx" that cannot be resolved to a single, unique
>> file/path/branch/tag/commit/etc is to display the following:
> ...
> So a possible implementation approach would be
>
>  - to let the current code do what it is doing
>
>  - except that you add new code immediately before the code that
>    issues 'xxx' did not match (i.e. the code already checked that
>    'xxx' taken as a pathspec does not match anything, and about to
>    give the error message but hasn't done so just yet).
>
>  - your new code 
>
>    . checks if 'xxx' could be an attempt to refer to a rev but
>      insufficiently spelled out (e.g. both origin[12]/xxx exists, or
>      for a bonus point, a similarly named origin/xxy exists and
>      could be a typo).
>
>    . if the above check found something, then you report it and
>      terminate without complaining "pathspec 'xxx' did not
>      match..."
>
>    . on the other hand, if the above check did not find anything,
>      then you let the current code issue the same error message as
>      before.

I was sweeping my mailbox to collect loose ends that haven't been
tied down, and noticed that this topic does not seem to have reached
a conclusion.  Do we want to reboot the effort?  Or should we just
throw it in the #leftoverbits bin for now?

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-09-12  6:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 21:49 Suggestion: better error message when an ambiguous checkout is executed Mahmoud Al-Qudsi
2017-08-07 22:44 ` Junio C Hamano
2017-09-12  6:53   ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).