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

2013-01-16 Thread Michael J Gruber
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 g...@drmicha.warpmail.net
---
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.

 builtin/remote.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 937484d..ec07109 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1509,25 +1509,28 @@ static int get_one_entry(struct remote *remote, void 
*priv)
 {
struct string_list *list = priv;
struct strbuf url_buf = STRBUF_INIT;
-   const char **url;
-   int i, url_nr;
+   char *fetchurl0, *fetchurl1;
+   int i;
+
+   if (remote-pushurl_nr  0) {
+   fetchurl0 = fetch;
+   fetchurl1 = fetch fallback;
+   } else {
+   fetchurl0 = fetch/push;
+   fetchurl1 = fetch fallback/push;
+   }
 
-   if (remote-url_nr  0) {
-   strbuf_addf(url_buf, %s (fetch), remote-url[0]);
+   for (i = 0; i  remote-url_nr; i++) {
+   strbuf_addf(url_buf, %s (%s), remote-url[0], i ? fetchurl1 
: fetchurl0);
string_list_append(list, remote-name)-util =
strbuf_detach(url_buf, NULL);
-   } else
+   } /* else */
+   if (remote-url_nr == 0)
string_list_append(list, remote-name)-util = NULL;
-   if (remote-pushurl_nr) {
-   url = remote-pushurl;
-   url_nr = remote-pushurl_nr;
-   } else {
-   url = remote-url;
-   url_nr = remote-url_nr;
-   }
-   for (i = 0; i  url_nr; i++)
+
+   for (i = 0; i  remote-pushurl_nr; i++)
{
-   strbuf_addf(url_buf, %s (push), url[i]);
+   strbuf_addf(url_buf, %s (push), remote-pushurl[i]);
string_list_append(list, remote-name)-util =
strbuf_detach(url_buf, NULL);
}
-- 
1.8.1.1.456.g93e7b0a

--
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 g...@drmicha.warpmail.net
 ---
 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


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.base.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 url 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
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.base.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 url 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 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.base.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 url 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 Junio C Hamano
Michael J Gruber g...@drmicha.warpmail.net 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