Re: [PATCH] git-remote: distinguish between default and configured URLs

2013-01-16 Thread Junio C Hamano
Michael J Gruber  writes:

> In short, the separate listing is correct, but in this case there's no
> improvement in readability.

Yes, I think the "insteadOf" rewrite is a related but a separate
issue.

Is "remote -v" meant for diagnosing remote.origin.{url,pushurl} that
are misconfigured?

If not, the output just should just say the final outcome, i.e. what
destinations we will fetch from and push to, without cluttering the
output.

If on the other hand it is to help users debug their configuration,
the output also needs to explain exactly what made us decide those
destinations to use (e.g. to discover there was a leftover insteadof
in $HOME/.gitconfig the user forgot about).


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-remote: distinguish between default and configured URLs

2013-01-16 Thread John Keeping
On Wed, Jan 16, 2013 at 01:45:36PM +0100, Michael J Gruber wrote:
> John Keeping venit, vidit, dixit 16.01.2013 11:42:
>> On Wed, Jan 16, 2013 at 11:14:48AM +0100, Michael J Gruber wrote:
>>> The current output of "git remote -v" does not distinguish between
>>> explicitly configured push URLs and those coming from fetch lines.
>>>
>>> Revise the output so so that URLs are distinguished by their labels:
>>>
>>> (fetch): fetch config used for fetching only
>>> (fetch/push): fetch config used for fetching and pushing
>>> (fetch fallback/push): fetch config used for pushing only
>>> (fetch fallback): fetch config which is unused
>>> (push): push config used for pushing
>> 
>> How does this interact with url..pushInsteadOf?
>> 
>> I have a global rule to convert git:// URLs to ssh:// for pushing:
>> 
>> [url "g...@example.com:"]
>> pushInsteadOf = git://example.com/
>> 
>> With only a URL configured for a remote (no pushURL), I get (with Git
>> 1.8.1):
>> 
>> origin git://example.com/repository.git (fetch)
>> origin g...@example.com:repository.git (push)
>> 
>> From the original discussion in this thread, I think that if I did
>> "git remote set-url --add --push " it would replace my current push
>> URL, and the change to "(fetch/push)" doesn't help in this case.
>> 
>> Should there be special handling for pushInsteadOf here?
> 
> Thanks for pointing out this case.
> 
> The new code would still list this as two separate URLs because they
> really are; whether they come from two config entries or from one being
> subject to two different insteadof expansions is completely opaque to
> builtin/remote.c, unless remote.c learns to stick that additional info
> into struct remote somehow.

OK.  I like the new format, I was just wondering if it was a simple
enhancement to indicate a pushInsteadOf URL specially as well.

> In short, the separate listing is correct, but in this case there's no
> improvement in readability.
> 
> We could still say that (push)InsteadOf is a power feature and we want
> to help the "normal" case, but it's a bit half-assed. In the end we
> might even have to keep track of insteadof-expansions and display those
> also (i.e. "expanded from...")?

Given that it's not a trivial enhancement, I'd accept the argument that
someone who has configured pushInsteadOf can be expected to understand
the underlying git-config semantics of "git remote set-url".


John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-remote: distinguish between default and configured URLs

2013-01-16 Thread Michael J Gruber
John Keeping venit, vidit, dixit 16.01.2013 11:42:
> On Wed, Jan 16, 2013 at 11:14:48AM +0100, Michael J Gruber wrote:
>> The current output of "git remote -v" does not distinguish between
>> explicitly configured push URLs and those coming from fetch lines.
>>
>> Revise the output so so that URLs are distinguished by their labels:
>>
>> (fetch): fetch config used for fetching only
>> (fetch/push): fetch config used for fetching and pushing
>> (fetch fallback/push): fetch config used for pushing only
>> (fetch fallback): fetch config which is unused
>> (push): push config used for pushing
> 
> How does this interact with url..pushInsteadOf?
> 
> I have a global rule to convert git:// URLs to ssh:// for pushing:
> 
> [url "g...@example.com:"]
> pushInsteadOf = git://example.com/
> 
> With only a URL configured for a remote (no pushURL), I get (with Git
> 1.8.1):
> 
> origin git://example.com/repository.git (fetch)
> origin g...@example.com:repository.git (push)
> 
> From the original discussion in this thread, I think that if I did
> "git remote set-url --add --push " it would replace my current push
> URL, and the change to "(fetch/push)" doesn't help in this case.
> 
> Should there be special handling for pushInsteadOf here?
> 
> 
> John

Thanks for pointing out this case.

The new code would still list this as two separate URLs because they
really are; whether they come from two config entries or from one being
subject to two different insteadof expansions is completely opaque to
builtin/remote.c, unless remote.c learns to stick that additional info
into struct remote somehow.

In short, the separate listing is correct, but in this case there's no
improvement in readability.

We could still say that (push)InsteadOf is a power feature and we want
to help the "normal" case, but it's a bit half-assed. In the end we
might even have to keep track of insteadof-expansions and display those
also (i.e. "expanded from...")?

Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-remote: distinguish between default and configured URLs

2013-01-16 Thread John Keeping
On Wed, Jan 16, 2013 at 11:14:48AM +0100, Michael J Gruber wrote:
> The current output of "git remote -v" does not distinguish between
> explicitly configured push URLs and those coming from fetch lines.
> 
> Revise the output so so that URLs are distinguished by their labels:
> 
> (fetch): fetch config used for fetching only
> (fetch/push): fetch config used for fetching and pushing
> (fetch fallback/push): fetch config used for pushing only
> (fetch fallback): fetch config which is unused
> (push): push config used for pushing

How does this interact with url..pushInsteadOf?

I have a global rule to convert git:// URLs to ssh:// for pushing:

[url "g...@example.com:"]
pushInsteadOf = git://example.com/

With only a URL configured for a remote (no pushURL), I get (with Git
1.8.1):

origin git://example.com/repository.git (fetch)
origin g...@example.com:repository.git (push)

>From the original discussion in this thread, I think that if I did
"git remote set-url --add --push " it would replace my current push
URL, and the change to "(fetch/push)" doesn't help in this case.

Should there be special handling for pushInsteadOf here?


John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-remote: distinguish between default and configured URLs

2013-01-16 Thread Michael J Gruber
Michael J Gruber venit, vidit, dixit 16.01.2013 11:14:
> The current output of "git remote -v" does not distinguish between
> explicitly configured push URLs and those coming from fetch lines.
> 
> Revise the output so so that URLs are distinguished by their labels:
> 
> (fetch): fetch config used for fetching only
> (fetch/push): fetch config used for fetching and pushing
> (fetch fallback/push): fetch config used for pushing only
> (fetch fallback): fetch config which is unused
> (push): push config used for pushing
> 
> Signed-off-by: Michael J Gruber 
> ---
> Maybe something like this? It even seems to make the code in get_one_entry
> clearer.
> 
> I yet have to look at the tests, doc and other git-remote invocations.

Okay, so "git remote show remotename" copied the logic from "git remote
-v" but neither reused the code nor the output format. I guess we'd have
to implement the new logic and keep the old format? Refactoring would
require settling on a common format. Both outputs should be
ui-as-ui-can, but I'm afraid people are still grepping the output in
their scripts :(

Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html