Re: [PATCH] remote show: fix the -n option

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 9, 2008, 02:35 UTC
Message-ID: <alpine.DEB.1.00.0806090330490.1783@racer>
In-Reply-To: <484C901B.6000401@free.fr>

Hi,

On Mon, 9 Jun 2008, Olivier Marin wrote:

Show 11 quoted lines

> Johannes Schindelin a écrit :
> 
> >>  	struct option options[] = {
> >>  		OPT_GROUP("show specific options"),
> >> -		OPT__DRY_RUN(&dry_run),
> >> +		OPT_BOOLEAN('n', NULL, &no_query, "do not query remotes"),
> > 
> > Why?
> 
> Because I think it's something different. It's more like in "route -n" 
> than --dry-run in "patch --dry-run". Don't you think ?

No, I think that the information about stale branches and if the branches are up-to-date is missing. In that sense, it is not like "route -n" at all, which just skips one convenience step, but really a dry run, because the result is different (as opposed to differently displayed).

Show 6 quoted lines

> >> +			transport = transport_get(NULL, states.remote->url_nr > 0 ?
> > 
> > Please rewrap.
> 
> I'm not sure what you are talking about. Should I wrap after "NULL," 
> instead of "?"?

It is a too long line (way over 80 characters). So yes, you should wrap after the NULL here.

Show 9 quoted lines

> > Maybe we need two different values of got_states; not calling 
> > ls-remote and then showing things is okay, but calling ls-remote, 
> > getting an error and _then_ showing stuff is not okay, IMO.
> 
> In fact, it seems that get_ref_states() always return 0 or just die when 
> an error occur. And that transport_get_remote_refs() never return if 
> something goes wrong.
> 
> So, what about removing got_states and use !no_query instead ?

Hrmpf.  I did not mean to die() there...

Ciao, Dscho