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

From: Olivier Marin <dkr+ml.git@free.fr>
Date: Jun 9, 2008, 02:06 UTC
Message-ID: <484C901B.6000401@free.fr>
In-Reply-To: <alpine.DEB.1.00.0806090212270.1783@racer>

Johannes Schindelin a écrit :

> 
> Would have been nice to Cc: the author of the C rewrite.

Sorry for that, will do it next time.

Show 6 quoted lines

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

>> +			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 "?"?

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

Olivier.