Re: Bug? "git branch" failing to list all branches

On 3 June 2010 07:22, Jonathan Nieder [off-list ref] wrote:

quoted

I noticed this because "git branch -a" and "git branch -av"
unexpectedly gave a very different output.

Hmm --- so the error message must not have been very visible...

I have been working with such set of repositories that most of them
have one or two broken refs. I probably saw it but didn't care because
it was a known issue. If I had called git branch from a script and
piped the output somewhere while relying on exit status, I wouldn't
have noticed anything.

Will this make ‘git branch’ exit with status zero?  Scripts and people
with fancy prompts benefit from a nonzero exit status.

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.

However, if the git codebase has somewhere a global mechanism for
signalling errors by, for example, raising some flag when error() is
called, using that mechanism would be better, right?

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.

One other thought: this patch is line-wrapped, which means it cannot
be mechanically applied.  Documentation/SubmittingPatches has some
tips on sending a patch unmangled (and please also see the section
labelled "Sign your work").

Yeah, I have a kosher patch locally. I just copypasted the diff part
here for discussion.


Simo

-- 
() Today is the car of the cdr of your life.
/\ http://arc.pasp.de/