Am 03.06.2015 um 19:24 schrieb Junio C Hamano:
> Jeff King <p...@peff.net> writes:
>
>> My concern there would be risk of regression. I.e., that we would take
>> some case which used to error out and turn it into a silent noop. So I'd
>> prefer to keep the behavior the same, and just modify the error code
>> path. The end result is pretty similar to the user, because we obviously
>> cannot produce any actual output either way.
>
> Okay.
>
>> Something like:
>>
>> -- >8 --
>> Subject: log: diagnose empty HEAD more clearly
>>
>> If you init or clone an empty repository, the initial
>> message from running "git log" is not very friendly:
>>
>> $ git init
>> Initialized empty Git repository in /home/peff/foo/.git/
>> $ git log
>> fatal: bad default revision 'HEAD'
>>
>> Let's detect this situation and write a more friendly
>> message:
>>
>> $ git log
>> fatal: default revision 'HEAD' points to an unborn branch 'master'
>>
>> We also detect the case that 'HEAD' points to a broken ref;
>> this should be even less common, but is easy to see. Note
>> that we do not diagnose all possible cases. We rely on
>> resolve_ref, which means we do not get information about
>> complex cases. E.g., "--default master" would use dwim_ref
>> to find "refs/heads/master", but we notice only that
>> "master" does not exist. Similarly, a complex sha1
>> expression like "--default HEAD^2" will not resolve as a
>> ref.
>>
>> But that's OK. We fall back to the existing error message in
>> those cases, and they are unlikely to be used anyway.
>> Catching an empty or broken "HEAD" improves the common case,
>> and the other cases are not regressed.
>>
>> Signed-off-by: Jeff King <p...@peff.net>
>> ---
>> I'm not a big fan of the new error message, either, just because the
>> term "unborn" is also likely to be confusing to new users.
>>
>> We can also probably get rid of mentioning "HEAD" completely. It is
>> always the default unless the user has overridden it explicitly.
>
> I think that still mentioning "HEAD" goes directly against the
> reasoning you made in an earlier part of your message:
>
>> I think it is one of those cases where the message makes sense when you
>> know how git works. But a new user who does not even know what "HEAD" or
>> a ref actually is may find it a bit confusing. Saying "we can't run
>> git-log, your repository empty!" may seem redundantly obvious even to
>> such a new user. But saying "bad revision 'HEAD'" may leave them saying
>> "eh, what is HEAD and why is it bad?".
>
> and I agree with that reasoning: the user didn't say anything about
> "HEAD", so why complain about it?
>
> Which is what led me to say "Why are we defaulting to HEAD before
> checking if it even exists? Isn't that the root cause of this
> confusion? What happens if we stopped doing it?"
>
> And I think the "diagnose after finding that pending is empty and we
> were given 'def'" approach almost gives us the equivalent, which is
> why I said "Okay" above.
>
> If we can make it possible to tell if that "def" was given by the
> user (i.e. --default parameter) or by the machinery (e.g. "git log"
> and friends), then we can remove "almost" from the above sentence.
>
>> So we
>> could go with something like:
>>
>> fatal: your default branch 'master' does not contain any commits
>>
>> or something. I dunno. Bikeshed colors welcome. Adding:
>>
>> advise("try running 'git commit'");
>>
>> is probably too pedantic. ;)
>
> ;-)
>
> I am wondering if the attached is better, if only because it is of
> less impact. I have die() there to avoid the behaviour change, but
> if we are going to have another future version where we are willing
> to take incompatiblity for better end-user experience like we did at
> 2.0, I suspect turning it to warning() or even removing it might be
> a candidate for such a version. If you have one commit and ask
> "log", you get one commit back. If you have no commit and ask "log",
> I think it is OK to say that you should get nothing back without
> fuss.
>
> builtin/log.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 4c4e6be..3b568a1 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char
> **argv, const char *prefix,
> rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> argc = setup_revisions(argc, argv, rev, opt);
>
> + if (!rev->pending.nr && !opt->def)
> + die("you do not have a commit yet on your branch");
I am not a native english speaker but shouldn't this be:
"you do not have a commit on your branch yet"
??
> [...]
Stefan
--
----------------------------------------------------------------
/dev/random says: I bet you I could stop gambling.
python -c "print
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
