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

From: Olivier Marin <[email-masked]>
Date: Mon, 09 Jun 2008 04:06:19 +0200

Johannes Schindelin a écrit :
> 
> Would have been nice to Cc: the author of the C rewrite.

Sorry for that, will do it next time.

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