Re: [PATCH/RFC] revision: Show friendlier message.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Leila Muhtasib <muhtasib@xxxxxxxxx> writes:

> % mkdir test
> % cd test
> % git init
> Initialized empty Git repository in .git/
> % git log
> fatal: bad default revision 'HEAD'

I agree that the message, while it is technically correct and does
not deserve to be called a bug, can be made more friendly.

But setup_revisions() is a very low level routine that is used by
many plumbing commands, and it is a horrible layering violation to
tweak its behaviour based on argv[0] and also it is too inflexible
hack as a solution.  For example, don't you want to give a different
error message for "git log HEAD" with an explicit "HEAD" from the
command line?  Would you add a similar support for a command that is
not "log" by adding yet another strcmp() here?

Wouldn't it be a more reasonable alternative solution if you do this:

 1. Check if HEAD points at a commit _before_ setting opt->def to it
    in "git log" (and other end-user facing programs in the "log"
    family, possibly in cmd_log_init_finish() if that function is
    not called by a program where the current message should not
    change), and do _NOT_ set opt->def to it;

 2. Make setup_revisions() expose got_rev_arg to its callers
    (e.g. move it to struct rev_info);

 3. If you did not pass HEAD in opt->def and setup_revisions() said
    it did not "got_rev_arg", give whatever error message that you
    think is more user friendly.

That way, if HEAD points at a commit, or if HEAD doesn't point at a
commit but the user gave some existing commit from the command line,
you wouldn't see "bad default revision" at all.

And the most important part of this alternative is that the lower
level machinery does not have to _care_ about the reason why the
higher level passed a bad HEAD to it.

Personally, I tend to think that not saying anything and reporting
success, instead of any error message, would be the right thing to
do if you are changing the behaviour of this case anyway.

Hrm?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]