v3remote: add --fetch and --both options to set-url

> git remote set-url knew about the '--push' option to update just the
> pushurl, but it does not have a similar option for "update fetch URL and
> leave whatever was in place for the push URL".
>
> This patch adds support for a '--fetch' option which implements that use
> case in a backwards compatible way: if no --both, --push or --fetch
> options are given, then the push URL is modified too if it was not set
> before. This is the case since the push URL is implicitly based on the
> fetch URL.
>
> A '--both' option is added to make the command independent of previous
> pushurl settings. For the --add and --delete set operations, it will
> always set the push and/ or the fetch URLs. For the primary mode of
> operation (without --add or --delete), it will drop pushurl as the
> implicit push URL is the (fetch) URL.

> The documentation has also been updated and a missing '--push' option
> is added to the 'git remote -h' command.
>
> Tests are also added to verify the documented behavior.
>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
>
>  v2: fixed test case
>  v3: added --both option, changed --fetch --push behavior, added more tests for
>      --add/--delete cases, refactored to reduce duplication (not special-casing
>      add_mode without oldurl, just skip initially setting oldurl).
>
> Translators note: the help text gained more translatable strings and some
> strings got additional options.
> ---
> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> index cb103c8..bdd0305 100644
> --- a/Documentation/git-remote.txt
> +++ b/Documentation/git-remote.txt
> @@ -134,7 +134,16 @@ Changes URL remote points to. Sets first URL remote points to matching
>  regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
>  <oldurl> doesn't match any URL, error occurs and nothing is changed.
>  +
> -With '--push', push URLs are manipulated instead of fetch URLs.
> +With '--both', both the fetch and push URLs are manipulated.
> ++
> +With '--fetch', only fetch URLs are manipulated.
> ++
> +With '--push', only push URLs are manipulated.
> ++
> +If none of the '--both', '--fetch' or --push' options are given, then
> +'--both' applies only if no push URL was set before. Otherwise '--fetch'
> +is assumed for historical reasons. This default may change in the
> +future to '--both' to avoid surprises depending on the configuration.

> On Tue, Nov 25, 2014 at 6:48 AM, Peter Wu <peter@lekensteyn.nl> wrote:
> > git remote set-url knew about the '--push' option to update just the
> > pushurl, but it does not have a similar option for "update fetch URL and
> > leave whatever was in place for the push URL".
> >
> > This patch adds support for a '--fetch' option which implements that use
> > case in a backwards compatible way: if no --both, --push or --fetch
> > options are given, then the push URL is modified too if it was not set
> > before. This is the case since the push URL is implicitly based on the
> > fetch URL.
> >
> > A '--both' option is added to make the command independent of previous
> > pushurl settings. For the --add and --delete set operations, it will
> > always set the push and/ or the fetch URLs. For the primary mode of
> > operation (without --add or --delete), it will drop pushurl as the
> > implicit push URL is the (fetch) URL.
> 
> I've read (though perhaps not fully digested) the other email thread
> which led up to this version of the patch, as well as the
> documentation update in this patch, but I still don't understand the
> need for the --both option. Intuitively, I would expect that
> specifying --fetch and --push on the command line would have the same
> effect as the proposed --both option. Thus, why is a separate (and
> exclusive) --both option needed? Is it to reduce typing? What am I
> missing?

> > The documentation has also been updated and a missing '--push' option
> > is added to the 'git remote -h' command.
> >
> > Tests are also added to verify the documented behavior.
> >
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > ---
> >
> >  v2: fixed test case
> >  v3: added --both option, changed --fetch --push behavior, added more tests for
> >      --add/--delete cases, refactored to reduce duplication (not special-casing
> >      add_mode without oldurl, just skip initially setting oldurl).
> >
> > Translators note: the help text gained more translatable strings and some
> > strings got additional options.
> > ---
> > diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> > index cb103c8..bdd0305 100644
> > --- a/Documentation/git-remote.txt
> > +++ b/Documentation/git-remote.txt
> > @@ -134,7 +134,16 @@ Changes URL remote points to. Sets first URL remote points to matching
> >  regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
> >  <oldurl> doesn't match any URL, error occurs and nothing is changed.
> >  +
> > -With '--push', push URLs are manipulated instead of fetch URLs.
> > +With '--both', both the fetch and push URLs are manipulated.
> > ++
> > +With '--fetch', only fetch URLs are manipulated.
> > ++
> > +With '--push', only push URLs are manipulated.
> > ++
> > +If none of the '--both', '--fetch' or --push' options are given, then
> > +'--both' applies only if no push URL was set before. Otherwise '--fetch'
> > +is assumed for historical reasons. This default may change in the
> > +future to '--both' to avoid surprises depending on the configuration.
> 
> This explanation is somewhat tortuous. Assuming that the --both option
> is superfluous, perhaps this could be explained more clearly along
> these lines:
> 
> --- 8< ---
> `--fetch` changes fetch URLs, and --push changes push URLs. Specified
> together, both fetch and push URLs are changed.
> 
> For historical reasons, if neither --fetch nor --push is specified,
> then the fetch URL is changed, as well as the push URL if not already
> set.
> --- 8< ---

> git remote set-url knew about the '--push' option to update just the
> pushurl, but it does not have a similar option for "update fetch URL and
> leave whatever was in place for the push URL".
> 
> This patch adds support for a '--fetch' option which implements that use
> case in a backwards compatible way: if no --both, --push or --fetch
> options are given, then the push URL is modified too if it was not set
> before. This is the case since the push URL is implicitly based on the
> fetch URL.
> 
> A '--both' option is added to make the command independent of previous
> pushurl settings. For the --add and --delete set operations, it will
> always set the push and/ or the fetch URLs. For the primary mode of
> operation (without --add or --delete), it will drop pushurl as the
> implicit push URL is the (fetch) URL.
> 
> The documentation has also been updated and a missing '--push' option
> is added to the 'git remote -h' command.
> 
> Tests are also added to verify the documented behavior.
> 
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---

> git remote set-url knew about the '--push' option to update just the
> pushurl, but it does not have a similar option for "update fetch URL and
> leave whatever was in place for the push URL".
>
> This patch adds support for a '--fetch' option which implements that use
> case in a backwards compatible way: if no --both, --push or --fetch
> options are given, then the push URL is modified too if it was not set
> before. This is the case since the push URL is implicitly based on the
> fetch URL.
>
> A '--both' option is added to make the command independent of previous
> pushurl settings. For the --add and --delete set operations, it will
> always set the push and/ or the fetch URLs. For the primary mode of
> operation (without --add or --delete), it will drop pushurl as the
> implicit push URL is the (fetch) URL.
>
> The documentation has also been updated and a missing '--push' option
> is added to the 'git remote -h' command.
>
> Tests are also added to verify the documented behavior.
>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
>
>   v2: fixed test case
>   v3: added --both option, changed --fetch --push behavior, added more tests for
>       --add/--delete cases, refactored to reduce duplication (not special-casing
>       add_mode without oldurl, just skip initially setting oldurl).
>
> Translators note: the help text gained more translatable strings and some
> strings got additional options.
> ---
>   Documentation/git-remote.txt |  17 ++++--
>   builtin/remote.c             | 140 +++++++++++++++++++++++++++----------------
>   t/t5505-remote.sh            | 125 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 228 insertions(+), 54 deletions(-)
>
> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> index cb103c8..bdd0305 100644
> --- a/Documentation/git-remote.txt
> +++ b/Documentation/git-remote.txt
> @@ -15,9 +15,9 @@ SYNOPSIS
>   'git remote remove' <name>
>   'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
>   'git remote set-branches' [--add] <name> <branch>...
> -'git remote set-url' [--push] <name> <newurl> [<oldurl>]
> -'git remote set-url --add' [--push] <name> <newurl>
> -'git remote set-url --delete' [--push] <name> <url>
> +'git remote set-url' [--both | --fetch | --push] <name> <newurl> [<oldurl>]
> +'git remote set-url' [--both | --fetch | --push] '--add' <name> <newurl>
> +'git remote set-url' [--both | --fetch | --push] '--delete' <name> <url>
>   'git remote' [-v | --verbose] 'show' [-n] <name>...
>   'git remote prune' [-n | --dry-run] <name>...
>   'git remote' [-v | --verbose] 'update' [-p | --prune] [(<group> | <remote>)...]
> @@ -134,7 +134,16 @@ Changes URL remote points to. Sets first URL remote points to matching
>   regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
>   <oldurl> doesn't match any URL, error occurs and nothing is changed.
>   +
> -With '--push', push URLs are manipulated instead of fetch URLs.
> +With '--both', both the fetch and push URLs are manipulated.
> ++
> +With '--fetch', only fetch URLs are manipulated.
> ++
> +With '--push', only push URLs are manipulated.
> ++
> +If none of the '--both', '--fetch' or --push' options are given, then
> +'--both' applies only if no push URL was set before. Otherwise '--fetch'
> +is assumed for historical reasons. This default may change in the
> +future to '--both' to avoid surprises depending on the configuration.
>   +
>   With '--add', instead of changing some URL, new URL is added.
>   +
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 7f28f92..a250b23 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -18,9 +18,9 @@ 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 set-url [--push] <name> <newurl> [<oldurl>]"),
> -	N_("git remote set-url --add <name> <newurl>"),
> -	N_("git remote set-url --delete <name> <url>"),
> +	N_("git remote set-url [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
> +	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
> +	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
>   	NULL
>   };
>   
> @@ -66,9 +66,9 @@ static const char * const builtin_remote_update_usage[] = {
>   };
>   
>   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>"),
> -	N_("git remote set-url --delete <name> <url>"),
> +	N_("git remote set-url [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
> +	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
> +	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
>   	NULL
>   };
>   
> @@ -1503,21 +1503,35 @@ static int set_branches(int argc, const char **argv)
>   	return set_remote_branches(argv[0], argv + 1, add_mode);
>   }
>   
> +#define MODIFY_TYPE_FETCH       (1 << 0)
> +#define MODIFY_TYPE_PUSH        (1 << 1)
> +#define MODIFY_TYPE_BOTH        (MODIFY_TYPE_FETCH | MODIFY_TYPE_PUSH)
> +#define MODIFY_TYPE_HISTORIC    (MODIFY_TYPE_FETCH | (1 << 2))
> +
>   static int set_url(int argc, const char **argv)
>   {
> -	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
> +	int i, add_mode = 0, delete_mode = 0;
> +	int modify_type = MODIFY_TYPE_HISTORIC;
>   	int matches = 0, negative_matches = 0;
>   	const char *remotename = NULL;
>   	const char *newurl = NULL;
>   	const char *oldurl = NULL;
>   	struct remote *remote;
>   	regex_t old_regex;
> -	const char **urlset;
> -	int urlset_nr;
> -	struct strbuf name_buf = STRBUF_INIT;
> +	struct strbuf name_buf_fetch = STRBUF_INIT;
> +	struct strbuf name_buf_push = STRBUF_INIT;
>   	struct option options[] = {
> -		OPT_BOOL('\0', "push", &push_mode,
> -			 N_("manipulate push URLs")),
> +		OPT_GROUP(""),
> +		OPT_SET_INT('\0', "fetch", &modify_type,
> +			N_("manipulate just fetch URLs"),
> +			MODIFY_TYPE_FETCH),
> +		OPT_SET_INT('\0', "push", &modify_type,
> +			N_("manipulate just push URLs"),
> +			MODIFY_TYPE_PUSH),
> +		OPT_SET_INT('\0', "both", &modify_type,
> +			N_("manipulate both push and fetch URLs"),
> +			MODIFY_TYPE_BOTH),
> +		OPT_GROUP(""),
>   		OPT_BOOL('\0', "add", &add_mode,
>   			 N_("add URL")),
>   		OPT_BOOL('\0', "delete", &delete_mode,
> @@ -1535,7 +1549,8 @@ static int set_url(int argc, const char **argv)
>   
>   	remotename = argv[1];
>   	newurl = argv[2];
> -	if (argc > 3)
> +	/* The old URL is only meaningful for the primary non-set operation. */
> +	if (argc > 3 && !add_mode && !delete_mode)
>   		oldurl = argv[3];
>   
>   	if (delete_mode)
> @@ -1545,47 +1560,72 @@ static int set_url(int argc, const char **argv)
>   		die(_("No such remote '%s'"), remotename);
>   	remote = remote_get(remotename);
>   
> -	if (push_mode) {
> -		strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
> -		urlset = remote->pushurl;
> -		urlset_nr = remote->pushurl_nr;
> -	} else {
> -		strbuf_addf(&name_buf, "remote.%s.url", remotename);
> -		urlset = remote->url;
> -		urlset_nr = remote->url_nr;
> +	strbuf_addf(&name_buf_fetch, "remote.%s.url", remotename);
> +	strbuf_addf(&name_buf_push, "remote.%s.pushurl", remotename);
> +
> +	if (oldurl && !add_mode) {
> +		/* Old URL specified, or deletion. Demand that one matches. */
> +		if (regcomp(&old_regex, oldurl, REG_EXTENDED))
> +			die(_("Invalid old URL pattern: %s"), oldurl);
> +
> +		if (modify_type & MODIFY_TYPE_FETCH)
> +			for (i = 0; i < remote->url_nr; i++)
> +				if (!regexec(&old_regex, remote->url[i], 0, NULL, 0))
> +					matches++;
> +				else
> +					negative_matches++;
> +		if (delete_mode && !negative_matches && modify_type & MODIFY_TYPE_FETCH)
> +			die(_("Will not delete all non-push URLs"));
> +		if (modify_type & MODIFY_TYPE_PUSH)
> +			for (i = 0; i < remote->pushurl_nr; i++)
> +				if (!regexec(&old_regex, remote->pushurl[i], 0, NULL, 0))
> +					matches++;
> +				else
> +					negative_matches++;

> On 11/25/2014 12:48 PM, Peter Wu wrote:
> > git remote set-url knew about the '--push' option to update just the
> > pushurl, but it does not have a similar option for "update fetch URL and
> > leave whatever was in place for the push URL".
> >
> > This patch adds support for a '--fetch' option which implements that use
> > case in a backwards compatible way: if no --both, --push or --fetch
> > options are given, then the push URL is modified too if it was not set
> > before. This is the case since the push URL is implicitly based on the
> > fetch URL.
> >
> > A '--both' option is added to make the command independent of previous
> > pushurl settings. For the --add and --delete set operations, it will
> > always set the push and/ or the fetch URLs. For the primary mode of
> > operation (without --add or --delete), it will drop pushurl as the
> > implicit push URL is the (fetch) URL.
> >
> > The documentation has also been updated and a missing '--push' option
> > is added to the 'git remote -h' command.
> >
> > Tests are also added to verify the documented behavior.
> >
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > ---
> >
> >   v2: fixed test case
> >   v3: added --both option, changed --fetch --push behavior, added more tests for
> >       --add/--delete cases, refactored to reduce duplication (not special-casing
> >       add_mode without oldurl, just skip initially setting oldurl).
> >
> > Translators note: the help text gained more translatable strings and some
> > strings got additional options.
> > ---
> >   Documentation/git-remote.txt |  17 ++++--
> >   builtin/remote.c             | 140 +++++++++++++++++++++++++++----------------
> >   t/t5505-remote.sh            | 125 ++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 228 insertions(+), 54 deletions(-)
> >
> > diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> > index cb103c8..bdd0305 100644
> > --- a/Documentation/git-remote.txt
> > +++ b/Documentation/git-remote.txt
> > @@ -15,9 +15,9 @@ SYNOPSIS
> >   'git remote remove' <name>
> >   'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
> >   'git remote set-branches' [--add] <name> <branch>...
> > -'git remote set-url' [--push] <name> <newurl> [<oldurl>]
> > -'git remote set-url --add' [--push] <name> <newurl>
> > -'git remote set-url --delete' [--push] <name> <url>
> > +'git remote set-url' [--both | --fetch | --push] <name> <newurl> [<oldurl>]
> > +'git remote set-url' [--both | --fetch | --push] '--add' <name> <newurl>
> > +'git remote set-url' [--both | --fetch | --push] '--delete' <name> <url>
> >   'git remote' [-v | --verbose] 'show' [-n] <name>...
> >   'git remote prune' [-n | --dry-run] <name>...
> >   'git remote' [-v | --verbose] 'update' [-p | --prune] [(<group> | <remote>)...]
> > @@ -134,7 +134,16 @@ Changes URL remote points to. Sets first URL remote points to matching
> >   regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
> >   <oldurl> doesn't match any URL, error occurs and nothing is changed.
> >   +
> > -With '--push', push URLs are manipulated instead of fetch URLs.
> > +With '--both', both the fetch and push URLs are manipulated.
> > ++
> > +With '--fetch', only fetch URLs are manipulated.
> > ++
> > +With '--push', only push URLs are manipulated.
> > ++
> > +If none of the '--both', '--fetch' or --push' options are given, then
> > +'--both' applies only if no push URL was set before. Otherwise '--fetch'
> > +is assumed for historical reasons. This default may change in the
> > +future to '--both' to avoid surprises depending on the configuration.
> >   +
> >   With '--add', instead of changing some URL, new URL is added.
> >   +
> > diff --git a/builtin/remote.c b/builtin/remote.c
> > index 7f28f92..a250b23 100644
> > --- a/builtin/remote.c
> > +++ b/builtin/remote.c
> > @@ -18,9 +18,9 @@ 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 set-url [--push] <name> <newurl> [<oldurl>]"),
> > -	N_("git remote set-url --add <name> <newurl>"),
> > -	N_("git remote set-url --delete <name> <url>"),
> > +	N_("git remote set-url [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
> > +	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
> > +	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
> >   	NULL
> >   };
> >   
> > @@ -66,9 +66,9 @@ static const char * const builtin_remote_update_usage[] = {
> >   };
> >   
> >   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>"),
> > -	N_("git remote set-url --delete <name> <url>"),
> > +	N_("git remote set-url [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
> > +	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
> > +	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
> >   	NULL
> >   };
> >   
> > @@ -1503,21 +1503,35 @@ static int set_branches(int argc, const char **argv)
> >   	return set_remote_branches(argv[0], argv + 1, add_mode);
> >   }
> >   
> > +#define MODIFY_TYPE_FETCH       (1 << 0)
> > +#define MODIFY_TYPE_PUSH        (1 << 1)
> > +#define MODIFY_TYPE_BOTH        (MODIFY_TYPE_FETCH | MODIFY_TYPE_PUSH)
> > +#define MODIFY_TYPE_HISTORIC    (MODIFY_TYPE_FETCH | (1 << 2))
> > +
> >   static int set_url(int argc, const char **argv)
> >   {
> > -	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
> > +	int i, add_mode = 0, delete_mode = 0;
> > +	int modify_type = MODIFY_TYPE_HISTORIC;
> >   	int matches = 0, negative_matches = 0;
> >   	const char *remotename = NULL;
> >   	const char *newurl = NULL;
> >   	const char *oldurl = NULL;
> >   	struct remote *remote;
> >   	regex_t old_regex;
> > -	const char **urlset;
> > -	int urlset_nr;
> > -	struct strbuf name_buf = STRBUF_INIT;
> > +	struct strbuf name_buf_fetch = STRBUF_INIT;
> > +	struct strbuf name_buf_push = STRBUF_INIT;
> >   	struct option options[] = {
> > -		OPT_BOOL('\0', "push", &push_mode,
> > -			 N_("manipulate push URLs")),
> > +		OPT_GROUP(""),
> > +		OPT_SET_INT('\0', "fetch", &modify_type,
> > +			N_("manipulate just fetch URLs"),
> > +			MODIFY_TYPE_FETCH),
> > +		OPT_SET_INT('\0', "push", &modify_type,
> > +			N_("manipulate just push URLs"),
> > +			MODIFY_TYPE_PUSH),
> > +		OPT_SET_INT('\0', "both", &modify_type,
> > +			N_("manipulate both push and fetch URLs"),
> > +			MODIFY_TYPE_BOTH),
> > +		OPT_GROUP(""),
> >   		OPT_BOOL('\0', "add", &add_mode,
> >   			 N_("add URL")),
> >   		OPT_BOOL('\0', "delete", &delete_mode,
> > @@ -1535,7 +1549,8 @@ static int set_url(int argc, const char **argv)
> >   
> >   	remotename = argv[1];
> >   	newurl = argv[2];
> > -	if (argc > 3)
> > +	/* The old URL is only meaningful for the primary non-set operation. */
> > +	if (argc > 3 && !add_mode && !delete_mode)
> >   		oldurl = argv[3];
> >   
> >   	if (delete_mode)
> > @@ -1545,47 +1560,72 @@ static int set_url(int argc, const char **argv)
> >   		die(_("No such remote '%s'"), remotename);
> >   	remote = remote_get(remotename);
> >   
> > -	if (push_mode) {
> > -		strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
> > -		urlset = remote->pushurl;
> > -		urlset_nr = remote->pushurl_nr;
> > -	} else {
> > -		strbuf_addf(&name_buf, "remote.%s.url", remotename);
> > -		urlset = remote->url;
> > -		urlset_nr = remote->url_nr;
> > +	strbuf_addf(&name_buf_fetch, "remote.%s.url", remotename);
> > +	strbuf_addf(&name_buf_push, "remote.%s.pushurl", remotename);
> > +
> > +	if (oldurl && !add_mode) {
> > +		/* Old URL specified, or deletion. Demand that one matches. */
> > +		if (regcomp(&old_regex, oldurl, REG_EXTENDED))
> > +			die(_("Invalid old URL pattern: %s"), oldurl);
> > +
> > +		if (modify_type & MODIFY_TYPE_FETCH)
> > +			for (i = 0; i < remote->url_nr; i++)
> > +				if (!regexec(&old_regex, remote->url[i], 0, NULL, 0))
> > +					matches++;
> > +				else
> > +					negative_matches++;
> > +		if (delete_mode && !negative_matches && modify_type & MODIFY_TYPE_FETCH)
> > +			die(_("Will not delete all non-push URLs"));
> > +		if (modify_type & MODIFY_TYPE_PUSH)
> > +			for (i = 0; i < remote->pushurl_nr; i++)
> > +				if (!regexec(&old_regex, remote->pushurl[i], 0, NULL, 0))
> > +					matches++;
> > +				else
> > +					negative_matches++;
> There are 2 warning  "dangling else", line 1570 and 1578
> I think we should use:
> 
> 			for (i = 0; i < remote->pushurl_nr; i++) {
> 				if (!regexec(&old_regex, remote->pushurl[i], 0, NULL, 0))
> 					matches++;
> 				else
> 					negative_matches++;
> 			}
> 

> > There are 2 warning  "dangling else", line 1570 and 1578
> > I think we should use:
> > 
> > 			for (i = 0; i < remote->pushurl_nr; i++) {
> > 				if (!regexec(&old_regex, remote->pushurl[i], 0, NULL, 0))
> > 					matches++;
> > 				else
> > 					negative_matches++;
> > 			}
> > 
> 
> Good catch, strange enough I did not get any warnings from my compiler
> or Clang's static analyzer. I have submitted a new patch which treats
> the accumulated comments.