On 3 June 2010 07:22, Jonathan Nieder <jrnieder@gmail.com> wrote:
quoted text
>> 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.

quoted text
> 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?

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

quoted text
> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html