Minor bug report

Minor bug report

[Tummala Dhanvi]

When we do create a new empty git repo using git init or create a
orphan branch and do a git log then I am getting an error saying that
fatal: bad default revision 'HEAD'

Well the error should have been something like no commits to show
either the branch is orphan / you didn't make any commits in the new
repo

I would like to fix the trival bug myself can some one point me in the
right direction to fix it

Re: Minor bug report

[Jeff King]

On Wed, Jun 03, 2015 at 11:24:19AM +0530, Tummala Dhanvi wrote:

> When we do create a new empty git repo using git init or create a
> orphan branch and do a git log then I am getting an error saying that
> fatal: bad default revision 'HEAD'
> 
> Well the error should have been something like no commits to show
> either the branch is orphan / you didn't make any commits in the new
> repo
> 
> I would like to fix the trival bug myself can some one point me in the
> right direction to fix it

Here are some prior discussions:

  http://thread.gmane.org/gmane.comp.version-control.git/75692

  http://thread.gmane.org/gmane.comp.version-control.git/200504

I just skimmed through them, but I expect the most desirable solution
would involve:

  1. We still die(), but just improve the error message (so we don't
     have any regressions for people expecting "git log" to fail).

  2. We use the message only when pointing to an unborn branch, and not
     on other errors. The simplest way to do this is probably to make an
     extra call to resolve_ref() in the error code-path.

-Peff

Re: Minor bug report

[Junio C Hamano]

On Tue, Jun 2, 2015 at 11:20 PM, Jeff King <peff@peff.net> wrote:
>
> Here are some prior discussions:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/75692
>
>   http://thread.gmane.org/gmane.comp.version-control.git/200504
>
> I just skimmed through them, but I expect the most desirable solution
> would involve:
>
>   1. We still die(), but just improve the error message (so we don't
>      have any regressions for people expecting "git log" to fail).
>
>   2. We use the message only when pointing to an unborn branch, and not
>      on other errors. The simplest way to do this is probably to make an
>      extra call to resolve_ref() in the error code-path.
>
> -Peff

I am kind of surprised after reading these two threads that my
take on this issue has changed over time, as my knee-jerk
reaction before reading them was the opposite, something
along the lines of "This is only immediately after 'git init'; why
bother?". Or depending on my mood, that "How stupid do you
have to be..." sounds exactly like a message I may advocate
for us to send. Perhaps I grew more bitter with age.

Jokes aside, I wonder why we did not pursue that "default to
HEAD only when HEAD points at a branch that exists"
approach, with the definition of "exists" being "either a file
exists under the $GIT_DIR/refs/heads directory with any
(including corrupt) contents, or an entry in the packed refs
file exists with any (including corrupt) contents". With or
without the error exit and warning, that sounds like a very
sensible way to set the default, at least to me.

[PATCH] log: diagnose empty HEAD more clearly

[Jeff King]

On Tue, Jun 02, 2015 at 11:48:30PM -0700, Junio C Hamano wrote:

> I am kind of surprised after reading these two threads that my
> take on this issue has changed over time, as my knee-jerk
> reaction before reading them was the opposite, something
> along the lines of "This is only immediately after 'git init'; why
> bother?". Or depending on my mood, that "How stupid do you
> have to be..." sounds exactly like a message I may advocate
> for us to send. Perhaps I grew more bitter with age.

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?".

> Jokes aside, I wonder why we did not pursue that "default to
> HEAD only when HEAD points at a branch that exists"
> approach, with the definition of "exists" being "either a file
> exists under the $GIT_DIR/refs/heads directory with any
> (including corrupt) contents, or an entry in the packed refs
> file exists with any (including corrupt) contents". With or
> without the error exit and warning, that sounds like a very
> sensible way to set the default, at least to me.

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.

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 <peff@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. 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. ;)

 revision.c     | 19 ++++++++++++++++++-
 t/t4202-log.sh | 11 +++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index 7ddbaa0..775df8b 100644
--- a/revision.c
+++ b/revision.c
@@ -2170,6 +2170,23 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	return 1;
 }
 
+static void NORETURN diagnose_missing_default(const char *def)
+{
+	unsigned char sha1[20];
+	int flags;
+	const char *refname;
+
+	refname = resolve_ref_unsafe(def, 0, sha1, &flags);
+	if (!(flags & REF_ISSYMREF))
+		die(_("bad default revision '%s'"), def);
+	if (flags & REF_ISBROKEN)
+		die(_("default revision '%s' points to a broken branch"), def);
+
+	skip_prefix(refname, "refs/heads/", &refname);
+	die(_("default revision '%s' points to an unborn branch '%s'"),
+	    def, refname);
+}
+
 /*
  * Parse revision information, filling in the "rev_info" structure,
  * and removing the used arguments from the argument list.
@@ -2299,7 +2316,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		struct object *object;
 		struct object_context oc;
 		if (get_sha1_with_context(revs->def, 0, sha1, &oc))
-			die("bad default revision '%s'", revs->def);
+			diagnose_missing_default(revs->def);
 		object = get_reference(revs, revs->def, sha1, 0);
 		add_pending_object_with_mode(revs, object, revs->def, oc.mode);
 	}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 1b2e981..71d8326 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -871,4 +871,15 @@ test_expect_success 'log --graph --no-walk is forbidden' '
 	test_must_fail git log --graph --no-walk
 '
 
+test_expect_success 'log diagnoses bogus HEAD' '
+	git init empty &&
+	test_must_fail git -C empty log 2>stderr &&
+	test_i18ngrep unborn stderr &&
+	echo 1234abcd >empty/.git/refs/heads/master &&
+	test_must_fail git -C empty log 2>stderr &&
+	test_i18ngrep broken stderr &&
+	test_must_fail git -C empty log --default totally-bogus 2>stderr &&
+	test_i18ngrep bad.default.revision stderr
+'
+
 test_done
-- 
2.4.2.745.g0aa058d

Re: Minor bug report

[Dennis Kaarsemaker]

On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote:
> 
> I am kind of surprised after reading these two threads that my
> take on this issue has changed over time, as my knee-jerk
> reaction before reading them was the opposite, something
> along the lines of "This is only immediately after 'git init'; why
> bother?". Or depending on my mood, that "How stupid do you
> have to be..." sounds exactly like a message I may advocate
> for us to send. Perhaps I grew more bitter with age.

The "fatal: Failed to resolve 'HEAD' as a valid ref." message, closely
related to the "fatal: bad default revision 'HEAD'" message that started
this thread just came by in #git with the following situation:

$ git init
$ git add .
# Oops, didn't want to add foo.xyz
$ git reset foo.xyz
fatal: Failed to resolve 'HEAD' as a valid ref.

The solution there is simple, git rm --cached, but I think git could
produce more helpful messages when a repo is empty.

I think you are growing bitter with age ;)

-- 
Dennis Kaarsemaker
www.kaarsemaker.net

Re: [PATCH] log: diagnose empty HEAD more clearly

[Junio C Hamano]

Jeff King <peff@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 <peff@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");
+
 	/* Any arguments at this point are not recognized */
 	if (argc > 1)
 		die(_("unrecognized argument: %s"), argv[1]);
@@ -404,6 +407,14 @@ static int git_log_config(const char *var, const char *value, void *cb)
 	return git_diff_ui_config(var, value, cb);
 }
 
+static void default_to_head_if_exists(struct setup_revision_opt *opt)
+{
+	unsigned char unused[20];
+
+	if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL))
+		opt->def = "HEAD";
+}
+
 int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info rev;
@@ -416,7 +427,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 	rev.diff = 1;
 	rev.simplify_history = 0;
 	memset(&opt, 0, sizeof(opt));
-	opt.def = "HEAD";
+	default_to_head_if_exists(&opt);
 	opt.revarg_opt = REVARG_COMMITTISH;
 	cmd_log_init(argc, argv, prefix, &rev, &opt);
 	if (!rev.diffopt.output_format)
@@ -530,7 +541,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 	rev.diffopt.stat_width = -1; 	/* Scale to real terminal size */
 
 	memset(&opt, 0, sizeof(opt));
-	opt.def = "HEAD";
+	default_to_head_if_exists(&opt);
 	opt.tweak = show_rev_tweak_rev;
 	cmd_log_init(argc, argv, prefix, &rev, &opt);
 
@@ -607,7 +618,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 	init_reflog_walk(&rev.reflog_info);
 	rev.verbose_header = 1;
 	memset(&opt, 0, sizeof(opt));
-	opt.def = "HEAD";
+	default_to_head_if_exists(&opt);
 	cmd_log_init_defaults(&rev);
 	rev.abbrev_commit = 1;
 	rev.commit_format = CMIT_FMT_ONELINE;
@@ -629,7 +640,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 	init_revisions(&rev, prefix);
 	rev.always_show_header = 1;
 	memset(&opt, 0, sizeof(opt));
-	opt.def = "HEAD";
+	default_to_head_if_exists(&opt);
 	opt.revarg_opt = REVARG_COMMITTISH;
 	cmd_log_init(argc, argv, prefix, &rev, &opt);
 	return cmd_log_walk(&rev);

Re: [PATCH] log: diagnose empty HEAD more clearly

[Stefan Näwe]

Am 03.06.2015 um 19:24 schrieb Junio C Hamano:
> Jeff King <peff@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 <peff@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

Re: Minor bug report

[Jeff King]

On Wed, Jun 03, 2015 at 05:39:14PM +0200, Dennis Kaarsemaker wrote:

> On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote:
> > 
> > I am kind of surprised after reading these two threads that my
> > take on this issue has changed over time, as my knee-jerk
> > reaction before reading them was the opposite, something
> > along the lines of "This is only immediately after 'git init'; why
> > bother?". Or depending on my mood, that "How stupid do you
> > have to be..." sounds exactly like a message I may advocate
> > for us to send. Perhaps I grew more bitter with age.
> 
> The "fatal: Failed to resolve 'HEAD' as a valid ref." message, closely
> related to the "fatal: bad default revision 'HEAD'" message that started
> this thread just came by in #git with the following situation:
> 
> $ git init
> $ git add .
> # Oops, didn't want to add foo.xyz
> $ git reset foo.xyz
> fatal: Failed to resolve 'HEAD' as a valid ref.
> 
> The solution there is simple, git rm --cached, but I think git could
> produce more helpful messages when a repo is empty.

Yeah, I think there are a lot of places we could handle unborn branches
better. We've slowly been smoothing these rough edges over the years
(usually by using the empty tree when we wanted HEAD to be a tree-ish).

Patches welcome. :)

-Peff

Re: [PATCH] log: diagnose empty HEAD more clearly

[Jeff King]

On Wed, Jun 03, 2015 at 10:24:02AM -0700, Junio C Hamano wrote:

> 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.

My thought was that we would not stop dying (for compatibility), and
that if we keep dying, it is effectively the same as what I proposed
(perhaps slightly worse, in the sense that we do not diagnose "--default
foo" as thoroughly, but effectively the same in that basically nobody
uses "--default" in the first place).

But if you are OK to eventually stop dying, I think this line of
reasoning is OK.

> 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");

Do we want to mention the name of the branch here? I guess it does not
really matter. Perhaps "the current branch" would be better than "your
branch", though. Maybe:

  fatal: you do not have any commits yet on the current branch

This message hopefully goes away in the long run, but we'll have it for
a while.

> +static void default_to_head_if_exists(struct setup_revision_opt *opt)
> +{
> +	unsigned char unused[20];
> +
> +	if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL))
> +		opt->def = "HEAD";
> +}

This approach looks sane to me. Want to wrap it up with a commit
message and a test?

-Peff

Re: [PATCH] log: diagnose empty HEAD more clearly

[Jeff King]

On Thu, Jun 04, 2015 at 09:31:04AM +0200, Stefan Näwe wrote:

> > +	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"

Both are fine, as is:

  "you do not yet have a commit on your branch"

though I think yours is slightly more clear.

If you are wondering at the reason, "yet" is an adverb modifying "have".
So it may come before or after the verb. If we substitute a simpler verb
(that does not need a direct object "a commit") and drop the
prepositional phrase ("on your branch"), we can do either:

  - you do not yet program

  - you do not program yet

If we add back in the prepositional phrase (which is really acting as an
adverbial phrase, modifying the verb here), we can do it in either
order:

  - you do not program yet in the office

  - you do not program in the office yet

But the latter makes it more clear that the "yet" applies to "in the
office".

You can also add back in the object of the verb:

  - you do not yet program computers

but you would not want:

  - you do not program yet computers

because it splits the verb from its object.

</grammar rant>

-Peff

Re: [PATCH] log: diagnose empty HEAD more clearly

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> But if you are OK to eventually stop dying, I think this line of
> reasoning is OK.
>
>> 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");
>
> Do we want to mention the name of the branch here? I guess it does not
> really matter. Perhaps "the current branch" would be better than "your
> branch", though. Maybe:
>
>   fatal: you do not have any commits yet on the current branch
>
> This message hopefully goes away in the long run, but we'll have it for
> a while.
>
>> +static void default_to_head_if_exists(struct setup_revision_opt *opt)
>> +{
>> +	unsigned char unused[20];
>> +
>> +	if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL))
>> +		opt->def = "HEAD";
>> +}
>
> This approach looks sane to me. Want to wrap it up with a commit
> message and a test?

There is an unmerged attempt jc/log-missing-default-HEAD in my
"broken-out" repository https://github.com/gitster/git; I do agree
that we should spend more cycles after deciding to error out to
give a more detailed diagnosis, but I didn't have enough energy
to bring myself do that, so I've tentatively merged your version
instead of this one.