Simo Melenius wrote:
> My change doesn't change the current behaviour. At least git 1.7.0.4
> didn't give a nonzero exit status either. It would be good if it did.
>
> I could add that to my patch. I'm, however, unsure of what's the best
> way to communicate the error from append_ref() to cmd_branch(). A
> static variable in branch.c would of course do.
I see. You can use the cb_data argument to pass a result back:
struct append_ref_cb {
struct ref_list *ref_list;
int err;
};
A static variable might be simpler.
But that is a separate topic (for a separate patch). I can pick it up
if you don’t.
>> If I have 37 branches and an error is encountered looking up one of
>> them, with this patch the error message will scroll off the screen.
>> Is this worth worrying about? It depends on what the usual causes for
>> broken branch refs are and whether they require attention or can be
>> safely ignored.
>
> Since this only concerns the printing of branches, often for
> interactive viewing or bash completion, and does not affect any of the
> operations that modify the repository, I think it's sufficient that
> the error message is still readable from stderr for those who are
> interested.
Sorry, I must have been unclear. Let me illustrate with an example:
$ git branch
* (no branch)
cc/sequencer-rebase-i
db/svn-fe
error: branch 'dk/hash' does not point at a commit.
gp/debian-pu
gp/sid-patches
jl/gitk-submodule
jk/pull-rebase-message
jn/debian-build-depends
js/grep-open
ks/gitk-notes
nd/gitbox
nd/setup
rr/svn-remote
sb/sequencer-dev
sb/sequencer-rfc
...
If this scrolls on for too long, I will not see the message. Now if
the error is due to some kind of corruption or a broken script, I
would want to know about it right away, even if I can carry on with
my work without. So in that case, it would make sense to add at the
end:
fatal: some refs could not be read.
On the other hand, maybe there is some harmless process that often
creates these broken refs and such a message would be a nuisance.
Hoping that is clearer,
Jonathan
--
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