Re: [PATCH v3] remote: add get-url subcommand

2015-08-05 Thread Junio C Hamano
Ben Boeckel maths...@gmail.com writes:

 Expanding `insteadOf` is a part of ls-remote --url and there is no way
 to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
 to query both as well as a way to get all configured urls.

 Signed-off-by: Ben Boeckel maths...@gmail.com
 ---
  Documentation/git-remote.txt | 10 
  builtin/remote.c | 54 
 
  2 files changed, 64 insertions(+)

Changes to these two files look reasonable.

Don't you want to protect this feature from future breakage by
others by adding a couple of tests, though, to t/t5505?

Thanks.
--
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 v3] remote: add get-url subcommand

2015-08-05 Thread Ben Boeckel
On Wed, Aug 05, 2015 at 13:34:18 -0700, Junio C Hamano wrote:
 Changes to these two files look reasonable.
 
 Don't you want to protect this feature from future breakage by
 others by adding a couple of tests, though, to t/t5505?

Thanks, I've done so locally. It actually brings up this case:

$ git remote add someremote foo
$ git remote get-url --push someremote
fatal: no URLs configured for remote 'someremote'

Is it better to use:

remote = remote_get(remotename);
remote-pushurl;

if (remote-pushurl_nr)
remote-pushurl;
else
remote-url;

or:

remote = pushremote_get(remotename);
remote-pushurl;

? What is the actual difference between the two?

Thanks,

--Ben
--
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 v3] remote: add get-url subcommand

2015-08-05 Thread Junio C Hamano
Ben Boeckel maths...@gmail.com writes:

 On Wed, Aug 05, 2015 at 13:34:18 -0700, Junio C Hamano wrote:
 Changes to these two files look reasonable.
 
 Don't you want to protect this feature from future breakage by
 others by adding a couple of tests, though, to t/t5505?

 Thanks, I've done so locally. It actually brings up this case:

 $ git remote add someremote foo
 $ git remote get-url --push someremote
 fatal: no URLs configured for remote 'someremote'

 Is it better to use:

 remote = remote_get(remotename);
 remote-pushurl;

 if (remote-pushurl_nr)
 remote-pushurl;
 else
 remote-url;

 or:

 remote = pushremote_get(remotename);
 remote-pushurl;

 ? What is the actual difference between the two?

You tell me ;-)

The default remote based on the current branch is computed
differently based on the direction of the transfer, I think.

struct remote *remote_get(const char *name)
{
return remote_get_1(name, remote_for_branch);
}

struct remote *pushremote_get(const char *name)
{
return remote_get_1(name, pushremote_for_branch);
}

When you are not giving name explicitly, the second parameter to _1 
function is used to determine the name.

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


[PATCH v3] remote: add get-url subcommand

2015-08-04 Thread Ben Boeckel
Expanding `insteadOf` is a part of ls-remote --url and there is no way
to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
to query both as well as a way to get all configured urls.

Signed-off-by: Ben Boeckel maths...@gmail.com
---
 Documentation/git-remote.txt | 10 
 builtin/remote.c | 54 
 2 files changed, 64 insertions(+)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 4c6d6de..3c9bf45 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git remote remove' name
 'git remote set-head' name (-a | --auto | -d | --delete | branch)
 'git remote set-branches' [--add] name branch...
+'git remote get-url' [--push] [--all] name
 'git remote set-url' [--push] name newurl [oldurl]
 'git remote set-url --add' [--push] name newurl
 'git remote set-url --delete' [--push] name url
@@ -131,6 +132,15 @@ The named branches will be interpreted as if specified 
with the
 With `--add`, instead of replacing the list of currently tracked
 branches, adds to that list.
 
+'get-url'::
+
+Retrieves the URLs for a remote. Configurations for `insteadOf` and
+`pushInsteadOf` are expanded here. By default, only the first URL is listed.
++
+With '--push', push URLs are queried rather than fetch URLs.
++
+With '--all', all URLs for the remote will be listed.
+
 'set-url'::
 
 Changes URLs for the remote. Sets first URL for remote name that matches
diff --git a/builtin/remote.c b/builtin/remote.c
index f4a6ec9..904869a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,6 +18,7 @@ static const char * const builtin_remote_usage[] = {
N_(git remote prune [-n | --dry-run] name),
N_(git remote [-v | --verbose] update [-p | --prune] [(group | 
remote)...]),
N_(git remote set-branches [--add] name branch...),
+   N_(git remote get-url [--push] [--all] name),
N_(git remote set-url [--push] name newurl [oldurl]),
N_(git remote set-url --add name newurl),
N_(git remote set-url --delete name url),
@@ -65,6 +66,11 @@ static const char * const builtin_remote_update_usage[] = {
NULL
 };
 
+static const char * const builtin_remote_geturl_usage[] = {
+   N_(git remote get-url [--push] [--all] name),
+   NULL
+};
+
 static const char * const builtin_remote_seturl_usage[] = {
N_(git remote set-url [--push] name newurl [oldurl]),
N_(git remote set-url --add name newurl),
@@ -1497,6 +1503,52 @@ static int set_branches(int argc, const char **argv)
return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+static int get_url(int argc, const char **argv)
+{
+   int i, push_mode = 0, all_mode = 0;
+   const char *remotename = NULL;
+   struct remote *remote;
+   const char **url;
+   int url_nr;
+   struct option options[] = {
+   OPT_BOOL('\0', push, push_mode,
+N_(query push URLs rather than fetch URLs)),
+   OPT_BOOL('\0', all, all_mode,
+N_(return all URLs)),
+   OPT_END()
+   };
+   argc = parse_options(argc, argv, NULL, options, 
builtin_remote_geturl_usage, 0);
+
+   if (argc != 1)
+   usage_with_options(builtin_remote_geturl_usage, options);
+
+   remotename = argv[0];
+
+   if (!remote_is_configured(remotename))
+   die(_(No such remote '%s'), remotename);
+   remote = remote_get(remotename);
+
+   if (push_mode) {
+   url = remote-pushurl;
+   url_nr = remote-pushurl_nr;
+   } else {
+   url = remote-url;
+   url_nr = remote-url_nr;
+   }
+
+   if (!url_nr)
+   die(_(no URLs configured for remote '%s'), remotename);
+
+   if (all_mode) {
+   for (i = 0; i  url_nr; i++)
+   printf_ln(%s, url[i]);
+   } else {
+   printf_ln(%s, *url);
+   }
+
+   return 0;
+}
+
 static int set_url(int argc, const char **argv)
 {
int i, push_mode = 0, add_mode = 0, delete_mode = 0;
@@ -1606,6 +1658,8 @@ int cmd_remote(int argc, const char **argv, const char 
*prefix)
result = set_head(argc, argv);
else if (!strcmp(argv[0], set-branches))
result = set_branches(argc, argv);
+   else if (!strcmp(argv[0], get-url))
+   result = get_url(argc, argv);
else if (!strcmp(argv[0], set-url))
result = set_url(argc, argv);
else if (!strcmp(argv[0], show))
-- 
2.1.0

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