[PATCH 0/3] for-each-ref: add :remote-ref and :remote-name specifiers

[PATCH 0/3] for-each-ref: add :remote-ref and :remote-name specifiers

[Johannes Schindelin]

This introduces support for

	git for-each-ref \
		--format="%(merge:remote-name),%(merge:remote-ref)"
	git for-each-ref \
		--format="%(push:remote-name),%(push:remote-ref)"

to figure out the remote nickname as well as the name of the corresponding
branch on the remote.

Note: the `%(push:remote-name)` placeholder is only interpolated by the value
of `branch.<name>.pushRemote`; unlike `git push`, it does not fall back to
`branch.<name>.remote`. Likewise, `%(push:remote-ref)` interpolates to the
empty string unless `remote.<nick>.pushRefs` is configured.

This is useful for third-party tools that need to know this type of information
for tons of branches.


J Wyman (1):
  for-each-ref: let upstream/push optionally report merge name.

Johannes Schindelin (2):
  for-each-ref: let upstream/push optionally report the remote name
  for-each-ref: test :remote-name and :remote-ref

 Documentation/git-for-each-ref.txt | 19 +++++++++++--------
 ref-filter.c                       | 35 +++++++++++++++++++++++++++++++----
 remote.c                           | 30 ++++++++++++++++++++++++++++++
 remote.h                           |  2 ++
 t/t6300-for-each-ref.sh            | 32 ++++++++++++++++++++++++++++++++
 5 files changed, 106 insertions(+), 12 deletions(-)


base-commit: ea220ee40cbb03a63ebad2be902057bf742492fd
Published-As: https://github.com/dscho/git/releases/tag/ref-filter-remote-name-v1
Fetch-It-Via: git fetch https://github.com/dscho/git ref-filter-remote-name-v1
-- 
2.14.2.windows.1

[PATCH 1/3] for-each-ref: let upstream/push optionally report the remote name

[Johannes Schindelin]

There are times when e.g. scripts want to know not only the name of the
upstream branch on the remote repository, but also the name of the
remote.

This patch offers the new suffix :remote for the upstream and for the
push atoms, allowing to show exactly that. Example:

	$ cat .git/config
	...
	[remote "origin"]
		url = https://where.do.we.come/from
		fetch = refs/heads/*:refs/remote/origin/*
	[remote "hello-world"]
		url = https://hello.world/git
		fetch = refs/heads/*:refs/remote/origin/*
		pushURL = hello.world:git
		push = refs/heads/*:refs/heads/*
	[branch "master"]
		remote = origin
		pushRemote = hello-world
	...

	$ git for-each-ref \
		--format='%(upstream) %(upstream:remote) %(push:remote)' \
		refs/heads/master
	refs/remotes/origin/master origin hello-world

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-for-each-ref.txt | 16 +++++++++-------
 ref-filter.c                       | 25 +++++++++++++++++++++----
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 66b4e0a4050..776368ee531 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -140,17 +140,19 @@ upstream::
 	(behind), "<>" (ahead and behind), or "=" (in sync). `:track`
 	also prints "[gone]" whenever unknown upstream ref is
 	encountered. Append `:track,nobracket` to show tracking
-	information without brackets (i.e "ahead N, behind M").  Has
-	no effect if the ref does not have tracking information
-	associated with it.  All the options apart from `nobracket`
-	are mutually exclusive, but if used together the last option
-	is selected.
+	information without brackets (i.e "ahead N, behind M").  Also
+	respects `:remote-name` to state the name of the *remote* instead of
+	the ref.
++
+Has no effect if the ref does not have tracking information associated
+with it.  All the options apart from `nobracket` are mutually exclusive,
+but if used together the last option is selected.
 
 push::
 	The name of a local ref which represents the `@{push}`
 	location for the displayed ref. Respects `:short`, `:lstrip`,
-	`:rstrip`, `:track`, and `:trackshort` options as `upstream`
-	does. Produces an empty string if no `@{push}` ref is
+	`:rstrip`, `:track`, `:trackshort` and `:remote-name` options as
+	`upstream` does. Produces an empty string if no `@{push}` ref is
 	configured.
 
 HEAD::
diff --git a/ref-filter.c b/ref-filter.c
index bc591f4f3de..58d53c09390 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -76,7 +76,9 @@ static struct used_atom {
 		char color[COLOR_MAXLEN];
 		struct align align;
 		struct {
-			enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+			enum {
+				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
+			} option;
 			struct refname_atom refname;
 			unsigned int nobracket : 1;
 		} remote_ref;
@@ -156,6 +158,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 			atom->u.remote_ref.option = RR_TRACKSHORT;
 		else if (!strcmp(s, "nobracket"))
 			atom->u.remote_ref.nobracket = 1;
+		else if (!strcmp(s, "remote-name"))
+			atom->u.remote_ref.option = RR_REMOTE_NAME;
 		else {
 			atom->u.remote_ref.option = RR_REF;
 			refname_atom_parser_internal(&atom->u.remote_ref.refname,
@@ -1247,6 +1251,15 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = ">";
 		else
 			*s = "<>";
+	} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {
+		int explicit;
+		const char *remote = starts_with(atom->name, "push") ?
+			pushremote_for_branch(branch, &explicit) :
+			remote_for_branch(branch, &explicit);
+		if (explicit)
+			*s = xstrdup(remote);
+		else
+			*s = "";
 	} else
 		die("BUG: unhandled RR_* enum");
 }
@@ -1364,9 +1377,13 @@ static void populate_value(struct ref_array_item *ref)
 				continue;
 			branch = branch_get(branch_name);
 
-			refname = branch_get_push(branch, NULL);
-			if (!refname)
-				continue;
+			if (starts_with(name, "push:remote-"))
+				refname = NULL;
+			else {
+				refname = branch_get_push(branch, NULL);
+				if (!refname)
+					continue;
+			}
 			fill_remote_ref_details(atom, refname, branch, &v->s);
 			continue;
 		} else if (starts_with(name, "color:")) {
-- 
2.14.2.windows.1

Re: [PATCH 1/3] for-each-ref: let upstream/push optionally report the remote name

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> This patch offers the new suffix :remote for the upstream and for the
> push atoms, allowing to show exactly that.

Has the design changed since this description and examples were
written?  The documentation talks about ":remote-name".  I suspect
calling this ":remote" might be less hassle, as we do not have to
vet the current vocabulary to see if "remote-name" is the right way
to name a multi-word modifer (as opposed to "remotename", or
"remoteName", or "remote_name", or...).

> 	...
>
> 	$ git for-each-ref \
> 		--format='%(upstream) %(upstream:remote) %(push:remote)' \
> 		refs/heads/master

> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 66b4e0a4050..776368ee531 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -140,17 +140,19 @@ upstream::

Aside from "':remote-name'?  Do we really want that?" issue I
already mentioned, the changes to this file look sensible.

> diff --git a/ref-filter.c b/ref-filter.c
> index bc591f4f3de..58d53c09390 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -76,7 +76,9 @@ static struct used_atom {
>  		char color[COLOR_MAXLEN];
>  		struct align align;
>  		struct {
> -			enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
> +			enum {
> +				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
> +			} option;
>  			struct refname_atom refname;
>  			unsigned int nobracket : 1;
>  		} remote_ref;
> @@ -156,6 +158,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
>  			atom->u.remote_ref.option = RR_TRACKSHORT;
>  		else if (!strcmp(s, "nobracket"))
>  			atom->u.remote_ref.nobracket = 1;
> +		else if (!strcmp(s, "remote-name"))
> +			atom->u.remote_ref.option = RR_REMOTE_NAME;
>  		else {
>  			atom->u.remote_ref.option = RR_REF;
>  			refname_atom_parser_internal(&atom->u.remote_ref.refname,
> @@ -1247,6 +1251,15 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
>  			*s = ">";
>  		else
>  			*s = "<>";
> +	} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {
> +		int explicit;
> +		const char *remote = starts_with(atom->name, "push") ?
> +			pushremote_for_branch(branch, &explicit) :
> +			remote_for_branch(branch, &explicit);

I think "int explicit = 0;" is needed, as pushremote_for_branch()
does seem to expect that the "explicit" return parameter is
initialized by the caller.

> +		if (explicit)
> +			*s = xstrdup(remote);
> +		else
> +			*s = "";

This one is debatable.  For the user of "push" who wants to learn
where the branch is pushed to, the choice this patch makes is not
useful (i.e. such a user does not care where the definition comes
from; s/he only wants to know where the push goes).  For the user of
"git config" who wants to know how pushremote is configured, and
does not want to accept the fallback to remote, the behaviour of
this patch is useful.  I have a mild suspicion that majority of
users who would want to use this feature would fall into the former
category, i.e. users of "push", rather than the latter, i.e.
debuggers of their config files, and if that is true, we can drop
the whole "explicit" business to simplify the code and get a more
useful behaviour at the same time ;-)

> @@ -1364,9 +1377,13 @@ static void populate_value(struct ref_array_item *ref)
>  				continue;
>  			branch = branch_get(branch_name);
>  
> -			refname = branch_get_push(branch, NULL);
> -			if (!refname)
> -				continue;
> +			if (starts_with(name, "push:remote-"))

I am not sure what is going on here.  

Are we expecting "push:remote-name" here, or anticipating
"push:remote-bar" can also be given or what?

> +				refname = NULL;
> +			else {
> +				refname = branch_get_push(branch, NULL);
> +				if (!refname)
> +					continue;
> +			}
>  			fill_remote_ref_details(atom, refname, branch, &v->s);
>  			continue;
>  		} else if (starts_with(name, "color:")) {

Re: [PATCH 1/3] for-each-ref: let upstream/push optionally report the remote name

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> +	} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {
>> +		int explicit;
>> +		const char *remote = starts_with(atom->name, "push") ?
>> +			pushremote_for_branch(branch, &explicit) :
>> +			remote_for_branch(branch, &explicit);
>
> I think "int explicit = 0;" is needed, as pushremote_for_branch()
> does seem to expect that the "explicit" return parameter is
> initialized by the caller.

Not really. I misread these helper functions.  pushremote_for_branch()
ends up calling remote_for_branch() which does clear, so we are OK
without caller initializing it.

Re: [PATCH 1/3] for-each-ref: let upstream/push optionally report the remote name

[Johannes Schindelin]

Hi Junio,

On Wed, 4 Oct 2017, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > This patch offers the new suffix :remote for the upstream and for the
> > push atoms, allowing to show exactly that.
> 
> Has the design changed since this description and examples were
> written?

Obviously.

Also, I tried to address all of your concerns (or at least those that were
remaining in the end).

Thank you for your review,
Dscho

[PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.

[Johannes Schindelin]

From: J Wyman <jwyman@microsoft.com>

There are times when scripts want to know not only the name of the
push branch on the remote, but also the name of the branch as known
by the remote repository.

A useful example of this is with the push command where
`branch.<name>.merge` is useful as the <to> value. Example:

	$ git push <remote> <from>:<to>

This patch offers the new suffix :merge for the push atom, allowing to
show exactly that. Example:

	$ cat .git/config
	...
	[remote "origin"]
		url = https://where.do.we.come/from
		fetch = refs/heads/*:refs/remote/origin/*
	[branch "master"]
		remote = origin
		merge = refs/heads/master
	[branch "develop/with/topics"]
		remote = origin
		merge = refs/heads/develop/with/topics
	...

	$ git for-each-ref \
		--format='%(push) %(push:remote-ref)' \
		refs/heads
	refs/remotes/origin/master refs/heads/master
	refs/remotes/origin/develop/with/topics refs/heads/develop/with/topics
---
 Documentation/git-for-each-ref.txt | 11 ++++++-----
 ref-filter.c                       | 12 +++++++++++-
 remote.c                           | 30 ++++++++++++++++++++++++++++++
 remote.h                           |  2 ++
 4 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 776368ee531..ba1147a1223 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -141,8 +141,9 @@ upstream::
 	also prints "[gone]" whenever unknown upstream ref is
 	encountered. Append `:track,nobracket` to show tracking
 	information without brackets (i.e "ahead N, behind M").  Also
-	respects `:remote-name` to state the name of the *remote* instead of
-	the ref.
+	respects `:remote-name` to state the name of the *remote* instead
+	of the ref, and `:remote-ref` to state the name of the *reference*
+	as locally known by the remote.
 +
 Has no effect if the ref does not have tracking information associated
 with it.  All the options apart from `nobracket` are mutually exclusive,
@@ -151,9 +152,9 @@ but if used together the last option is selected.
 push::
 	The name of a local ref which represents the `@{push}`
 	location for the displayed ref. Respects `:short`, `:lstrip`,
-	`:rstrip`, `:track`, `:trackshort` and `:remote-name` options as
-	`upstream` does. Produces an empty string if no `@{push}` ref is
-	configured.
+	`:rstrip`, `:track`, `:trackshort`, `:remote-name`, and `:remote-ref`
+	options as `upstream` does. Produces an empty string if no `@{push}`
+	ref is configured.
 
 HEAD::
 	'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index 58d53c09390..4401e573b8a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -77,7 +77,7 @@ static struct used_atom {
 		struct align align;
 		struct {
 			enum {
-				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
+				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME, RR_REMOTE_REF
 			} option;
 			struct refname_atom refname;
 			unsigned int nobracket : 1;
@@ -160,6 +160,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 			atom->u.remote_ref.nobracket = 1;
 		else if (!strcmp(s, "remote-name"))
 			atom->u.remote_ref.option = RR_REMOTE_NAME;
+		else if (!strcmp(s, "remote-ref"))
+			atom->u.remote_ref.option = RR_REMOTE_REF;
 		else {
 			atom->u.remote_ref.option = RR_REF;
 			refname_atom_parser_internal(&atom->u.remote_ref.refname,
@@ -1260,6 +1262,14 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = xstrdup(remote);
 		else
 			*s = "";
+	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
+		int explicit, for_push = starts_with(atom->name, "push");
+		const char *merge = remote_ref_for_branch(branch, for_push,
+							  &explicit);
+		if (explicit)
+			*s = xstrdup(merge);
+		else
+			*s = "";
 	} else
 		die("BUG: unhandled RR_* enum");
 }
diff --git a/remote.c b/remote.c
index b220f0dfc61..2bdcfc280cd 100644
--- a/remote.c
+++ b/remote.c
@@ -675,6 +675,36 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
 	return remote_for_branch(branch, explicit);
 }
 
+const char *remote_ref_for_branch(struct branch *branch, int for_push,
+				  int *explicit)
+{
+	if (branch) {
+		if (!for_push) {
+			if (branch->merge_nr) {
+				if (explicit)
+					*explicit = 1;
+				return branch->merge_name[0];
+			}
+		} else {
+			const char *dst, *remote_name =
+				pushremote_for_branch(branch, NULL);
+			struct remote *remote = remote_get(remote_name);
+
+			if (remote && remote->push_refspec_nr &&
+			    (dst = apply_refspecs(remote->push,
+						  remote->push_refspec_nr,
+						  branch->refname))) {
+				if (explicit)
+					*explicit = 1;
+				return dst;
+			}
+		}
+	}
+	if (explicit)
+		*explicit = 0;
+	return "";
+}
+
 static struct remote *remote_get_1(const char *name,
 				   const char *(*get_default)(struct branch *, int *))
 {
diff --git a/remote.h b/remote.h
index 2ecf4c8c74c..1f6611be214 100644
--- a/remote.h
+++ b/remote.h
@@ -223,6 +223,8 @@ struct branch {
 struct branch *branch_get(const char *name);
 const char *remote_for_branch(struct branch *branch, int *explicit);
 const char *pushremote_for_branch(struct branch *branch, int *explicit);
+const char *remote_ref_for_branch(struct branch *branch, int for_push,
+				  int *explicit);
 
 int branch_has_merge_config(struct branch *branch);
 int branch_merge_matches(struct branch *, int n, const char *);
-- 
2.14.2.windows.1

Re: [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> From: J Wyman <jwyman@microsoft.com>
>
> There are times when scripts want to know not only the name of the
> push branch on the remote, but also the name of the branch as known
> by the remote repository.
>
> A useful example of this is with the push command where
> `branch.<name>.merge` is useful as the <to> value. Example:
>
> 	$ git push <remote> <from>:<to>
>
> This patch offers the new suffix :merge for the push atom, allowing to
> show exactly that. Example:
>
> 	$ cat .git/config
> 	...
> 	[remote "origin"]
> 		url = https://where.do.we.come/from
> 		fetch = refs/heads/*:refs/remote/origin/*
> 	[branch "master"]
> 		remote = origin
> 		merge = refs/heads/master
> 	[branch "develop/with/topics"]
> 		remote = origin
> 		merge = refs/heads/develop/with/topics
> 	...
>
> 	$ git for-each-ref \
> 		--format='%(push) %(push:remote-ref)' \
> 		refs/heads
> 	refs/remotes/origin/master refs/heads/master
> 	refs/remotes/origin/develop/with/topics refs/heads/develop/with/topics

Ah, now it is clear that we _need_ to figure out how to spell
"multi-word" modifier to the format specifiers, as "push:remote"
would not let us differenciate the products of 1/3 and 2/3 X-<.

> ---

We need two Signed-off-by: lines at the end, one by Wyman and then
another by you in this order.

> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt

Changes to this file in this patch looks sane.

> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1260,6 +1262,14 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
>  			*s = xstrdup(remote);
>  		else
>  			*s = "";
> +	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
> +		int explicit, for_push = starts_with(atom->name, "push");
> +		const char *merge = remote_ref_for_branch(branch, for_push,
> +							  &explicit);

Having multiple variables defined like this _and_ initialized with
anything other than a trivial constant, is somewhat hard to follow.
Can we split the above more like:

		int explicit;
		int for_push = starts_with(...);
		const char *merge = remote_ref_for_branch(...);

Actually, if you did it this way, you do not even need "for_push":

		int explicit;
		const char *merge;

		merge = remote_ref_for_branch(branch,
					      starts_with(atom->name, "push"),
					      &explicit);

which may be a lot easier to follow.

By the way, the spirit of parsing used atoms first before the "learn
what we care about this single ref" function like the above are
called repeatedly for each ref is because we want to hoist the
overhead of doing the constant computation like "does the atom's
name begins with 'push'?" out of the latter.  

Can we add a field in atom->u.remote_ref to precompute this bit so
that we do not even have to say "Are we doing this for push?  Let's
see if the atom's name begins with 'push'" each time this function
is called for a ref?  That makes this function a lot more in line
with the spirit of the design of the subsystem, I would think.

This comment probably applies equally to both 1/3 and this one.

> +		if (explicit)
> +			*s = xstrdup(merge);
> +		else
> +			*s = "";

Here is the same "who are we trying to help---users who want to know
where their push goes, or users who are debugging where the push
destination is defined?" question.  I do not have a strong opinion
either way, but I think giving the end result with fallback (i.e.
not nullifying when the result is not explicit) may be easier to use
by "push" users.

> diff --git a/remote.c b/remote.c
> index b220f0dfc61..2bdcfc280cd 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -675,6 +675,36 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
>  	return remote_for_branch(branch, explicit);
>  }
>  
> +const char *remote_ref_for_branch(struct branch *branch, int for_push,
> +				  int *explicit)
> +{
> +	if (branch) {
> +		if (!for_push) {
> +			if (branch->merge_nr) {
> +				if (explicit)
> +					*explicit = 1;
> +				return branch->merge_name[0];
> +			}
> +		} else {
> +			const char *dst, *remote_name =
> +				pushremote_for_branch(branch, NULL);
> +			struct remote *remote = remote_get(remote_name);

Again,

			const char *dst, *remote_name;
                        struct remote *remote;

			remote_name = pushremote_for_branch(branch, NULL);
                        remote = remote_get(remote_name);

may be easier to follow, if only that it allows eyes to scan without
having to be distracted by jagged lines.

> +			if (remote && remote->push_refspec_nr &&
> +			    (dst = apply_refspecs(remote->push,
> +						  remote->push_refspec_nr,
> +						  branch->refname))) {
> +				if (explicit)
> +					*explicit = 1;
> +				return dst;
> +			}
> +		}
> +	}
> +	if (explicit)
> +		*explicit = 0;
> +	return "";
> +}

What the function does looks reasonable; the assignment in if()
condition looks a bit unfortunate, though.

Re: [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> +		if (explicit)
>> +			*s = xstrdup(merge);
>> +		else
>> +			*s = "";
>
> Here is the same "who are we trying to help---users who want to know
> where their push goes, or users who are debugging where the push
> destination is defined?" question.  I do not have a strong opinion
> either way, but I think giving the end result with fallback (i.e.
> not nullifying when the result is not explicit) may be easier to use
> by "push" users.

Now after thinking about it a bit more, I actually have a moderate
preference to doing it the way your patches do.  With programatic
%(if)%(else)%(end) support we acquired recently, the fallback can be
coded in the --format=... language if the user wanted to using the
"internal fallbacks, explicit==0, are ignored" behaviour that are
implemented by these two patches.  The reverse is not true.

I think the remaining points from my reviews are:

 - It would be better to compute precomputable stuff when used atoms
   are parsed, instead of running starts_with() in these functions;

 - We want to make sure there is no existing multi-word modifiers
   (in which case we can safely declare "multi-word" is the way we
   spell them from now on).  Or if there are existing ones, they
   already spell themselves as "multi-word".

   I have nothing against "remote-name"; I just want to make sure we
   are not making things inconsistent.  If there are only few (but
   non zero number of) multi-word modifiers that are not spelled
   "multi-word", as long as they are only few and their spelling are
   inferiour (e.g. concatenatedwords is much worse than
   concatenated-words), we could still declare "multi-word" is the
   right way to spell them going forward, declare that we will give
   them synonyms and deprecate the bad spelling out over time, and
   leave that plan as #leftover bits thing (i.e. not doing the
   deprecation of these other modifiers as part of this series.

   The only thing I want to happen in the scope of _this_ series, as
   due diligence, is to make sure we are happy with "multi-word",
   and also to know if we need a follow-up work (just yes/no,
   possibly with plans, but no actual work yet).

Thanks.

Re: [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> I think the remaining points from my reviews are:
>
>  - It would be better to compute precomputable stuff when used atoms
>    are parsed, instead of running starts_with() in these functions;

After reading the original (before these three patches) code for
populate_value(), I think reverting this mess back to how the thing
was originally envisioned to be would be outside the scope of this
addition.  It was way too messy before your series than I expected.

So, unless you or your collaborator feel like helping us to clean up
the codebase for better future, let's forget about that point, and
leave the cleaning up for now.

For those who want to help with the #leftoverbits, the first step of
cleaning up would be to introduce an enum field that sits next to
used_atom[i].name, and have the if/else if/... cascades use that
instead of these starts_with(name) to choose what value to grab from
the given ref.

[PATCH 3/3] for-each-ref: test :remote-name and :remote-ref

[Johannes Schindelin]

This not only prevents regressions, but also serves as documentation
what this new feature is expected to do.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6300-for-each-ref.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2274a4b7331..edc73dd79aa 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -675,4 +675,36 @@ test_expect_success 'Verify usage of %(symref:rstrip) atom' '
 	test_cmp expected actual
 '
 
+test_expect_success ':remote-name and :remote-ref' '
+	git init remote-tests &&
+	(
+		cd remote-tests &&
+		test_commit initial &&
+		git remote add from fifth.coffee:blub &&
+		git config branch.master.remote from &&
+		git config branch.master.merge refs/heads/stable &&
+		git remote add to southridge.audio:repo &&
+		git config remote.to.push "refs/heads/*:refs/heads/pushed/*" &&
+		git config branch.master.pushRemote to &&
+		for pair in "%(upstream)=refs/remotes/from/stable" \
+			"%(upstream:remote-name)=from" \
+			"%(upstream:remote-ref)=refs/heads/stable" \
+			"%(push)=refs/remotes/to/pushed/master" \
+			"%(push:remote-name)=to" \
+			"%(push:remote-ref)=refs/heads/pushed/master"
+		do
+			echo "${pair#*=}" >expect &&
+			git for-each-ref --format="${pair%=*}" \
+				refs/heads/master >actual &&
+			test_cmp expect actual
+		done &&
+		git branch push-simple &&
+		git config branch.push-simple.pushRemote from &&
+		actual="$(git for-each-ref \
+			--format="%(push:remote-name),%(push:remote-ref)" \
+			refs/heads/push-simple)" &&
+		test from, = "$actual"
+	)
+'
+
 test_done
-- 
2.14.2.windows.1

Re: [PATCH 0/3] for-each-ref: add :remote-ref and :remote-name specifiers

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Note: the `%(push:remote-name)` placeholder is only interpolated by the value
> of `branch.<name>.pushRemote`; unlike `git push`, it does not fall back to
> `branch.<name>.remote`. Likewise, `%(push:remote-ref)` interpolates to the
> empty string unless `remote.<nick>.pushRefs` is configured.

I think the reason why I had to spend more time than necessary on
the above point during my review on this (otherwise mostly well
done) topic was because of this note.  It says what it does, but
does not say why this behaviour is better than the other obvious
alternative.

In a reroll to address the remaining points, please update the log
message (not the cover, which won't be in the committed history) to
explain why we think this is a better choice than an obvious
alternative.  Let's help future readers of "git log".

Thanks.

[PATCH v2 0/3] for-each-ref: add :remoteref and :remotename specifiers

[Johannes Schindelin]

This introduces support for

	git for-each-ref \
		--format="%(merge:remotename),%(merge:remoteref)"
	git for-each-ref \
		--format="%(push:remotename),%(push:remoteref)"

to figure out the remote nickname as well as the name of the corresponding
branch on the remote.

Note: the `%(push:remotename)` placeholder is only interpolated by the value
of `branch.<name>.pushRemote`; unlike `git push`, it does not fall back to
`branch.<name>.remote`. Likewise, `%(push:remoteref)` interpolates to the
empty string unless `remote.<nick>.pushRefs` is configured.

This is useful for third-party tools that need to know this type of information
for tons of branches.

Changes since v1:

- added the missing sign-offs

- removed a full stop from the end of the oneline of 2/3 (quite frankly, I am
  surprised I was not slammed for that in the first round of reviews ;-))

- fixed stale commit message parts that were leftovers from previous, internal
  review rounds

- moved the added git-for-each-ref.txt documentation into its own paragraph

- renamed the :remote-name and :remote-ref suffixes into the dashless form
  :remotename and :remoteref, following the example of :trackshort

- clarified the motivation for not DWIMming in the commit messages

- added fields `push` and `push_remote` to the atom to precompute whether we
  are talking about a `push` atom and whether it has a suffix starting with
  `remote`, respectively

Note that this last point actually fixes a bug when the user specified more
than one suffix (`push:short,remotename`; even if that `short` has no effect,
the `remotename` should).


J Wyman (1):
  for-each-ref: let upstream/push optionally remote ref name

Johannes Schindelin (2):
  for-each-ref: let upstream/push optionally report the remote name
  for-each-ref: test :remotename and :remoteref

 Documentation/git-for-each-ref.txt | 20 ++++++++++-------
 ref-filter.c                       | 46 ++++++++++++++++++++++++++++++++------
 remote.c                           | 30 +++++++++++++++++++++++++
 remote.h                           |  2 ++
 t/t6300-for-each-ref.sh            | 32 ++++++++++++++++++++++++++
 5 files changed, 115 insertions(+), 15 deletions(-)


base-commit: 217f2767cbcb562872437eed4dec62e00846d90c
Published-As: https://github.com/dscho/git/releases/tag/ref-filter-remote-name-v2
Fetch-It-Via: git fetch https://github.com/dscho/git ref-filter-remote-name-v2

Interdiff vs v1:
 diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
 index ba1147a1223..22767025850 100644
 --- a/Documentation/git-for-each-ref.txt
 +++ b/Documentation/git-for-each-ref.txt
 @@ -140,10 +140,11 @@ upstream::
  	(behind), "<>" (ahead and behind), or "=" (in sync). `:track`
  	also prints "[gone]" whenever unknown upstream ref is
  	encountered. Append `:track,nobracket` to show tracking
 -	information without brackets (i.e "ahead N, behind M").  Also
 -	respects `:remote-name` to state the name of the *remote* instead
 -	of the ref, and `:remote-ref` to state the name of the *reference*
 -	as locally known by the remote.
 +	information without brackets (i.e "ahead N, behind M").
 ++
 +Also respects `:remotename` to state the name of the *remote* instead
 +of the ref, and `:remoteref` to state the name of the *reference* as
 +locally known by the remote.
  +
  Has no effect if the ref does not have tracking information associated
  with it.  All the options apart from `nobracket` are mutually exclusive,
 @@ -152,7 +153,7 @@ but if used together the last option is selected.
  push::
  	The name of a local ref which represents the `@{push}`
  	location for the displayed ref. Respects `:short`, `:lstrip`,
 -	`:rstrip`, `:track`, `:trackshort`, `:remote-name`, and `:remote-ref`
 +	`:rstrip`, `:track`, `:trackshort`, `:remotename`, and `:remoteref`
  	options as `upstream` does. Produces an empty string if no `@{push}`
  	ref is configured.
  
 diff --git a/ref-filter.c b/ref-filter.c
 index 696a8241408..b4b2c9b2190 100644
 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -80,7 +80,7 @@ static struct used_atom {
  				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME, RR_REMOTE_REF
  			} option;
  			struct refname_atom refname;
 -			unsigned int nobracket : 1;
 +			unsigned int nobracket : 1, push : 1, push_remote : 1;
  		} remote_ref;
  		struct {
  			enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option;
 @@ -139,6 +139,9 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
  	struct string_list params = STRING_LIST_INIT_DUP;
  	int i;
  
 +	if (!strcmp(atom->name, "push") || starts_with(atom->name, "push:"))
 +		atom->u.remote_ref.push = 1;
 +
  	if (!arg) {
  		atom->u.remote_ref.option = RR_REF;
  		refname_atom_parser_internal(&atom->u.remote_ref.refname,
 @@ -158,11 +161,13 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
  			atom->u.remote_ref.option = RR_TRACKSHORT;
  		else if (!strcmp(s, "nobracket"))
  			atom->u.remote_ref.nobracket = 1;
 -		else if (!strcmp(s, "remote-name"))
 +		else if (!strcmp(s, "remotename")) {
  			atom->u.remote_ref.option = RR_REMOTE_NAME;
 -		else if (!strcmp(s, "remote-ref"))
 +			atom->u.remote_ref.push_remote = 1;
 +		} else if (!strcmp(s, "remoteref")) {
  			atom->u.remote_ref.option = RR_REMOTE_REF;
 -		else {
 +			atom->u.remote_ref.push_remote = 1;
 +		} else {
  			atom->u.remote_ref.option = RR_REF;
  			refname_atom_parser_internal(&atom->u.remote_ref.refname,
  						     arg, atom->name);
 @@ -1253,7 +1258,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
  			*s = "<>";
  	} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {
  		int explicit;
 -		const char *remote = starts_with(atom->name, "push") ?
 +		const char *remote = atom->u.remote_ref.push ?
  			pushremote_for_branch(branch, &explicit) :
  			remote_for_branch(branch, &explicit);
  		if (explicit)
 @@ -1377,14 +1382,14 @@ static void populate_value(struct ref_array_item *ref)
  			if (refname)
  				fill_remote_ref_details(atom, refname, branch, &v->s);
  			continue;
 -		} else if (starts_with(name, "push")) {
 +		} else if (atom->u.remote_ref.push) {
  			const char *branch_name;
  			if (!skip_prefix(ref->refname, "refs/heads/",
  					 &branch_name))
  				continue;
  			branch = branch_get(branch_name);
  
 -			if (starts_with(name, "push:remote-"))
 +			if (atom->u.remote_ref.push_remote)
  				refname = NULL;
  			else {
  				refname = branch_get_push(branch, NULL);
 diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
 index edc73dd79aa..d9eb2be0256 100755
 --- a/t/t6300-for-each-ref.sh
 +++ b/t/t6300-for-each-ref.sh
 @@ -675,7 +675,7 @@ test_expect_success 'Verify usage of %(symref:rstrip) atom' '
  	test_cmp expected actual
  '
  
 -test_expect_success ':remote-name and :remote-ref' '
 +test_expect_success ':remotename and :remoteref' '
  	git init remote-tests &&
  	(
  		cd remote-tests &&
 @@ -687,11 +687,11 @@ test_expect_success ':remote-name and :remote-ref' '
  		git config remote.to.push "refs/heads/*:refs/heads/pushed/*" &&
  		git config branch.master.pushRemote to &&
  		for pair in "%(upstream)=refs/remotes/from/stable" \
 -			"%(upstream:remote-name)=from" \
 -			"%(upstream:remote-ref)=refs/heads/stable" \
 +			"%(upstream:remotename)=from" \
 +			"%(upstream:remoteref)=refs/heads/stable" \
  			"%(push)=refs/remotes/to/pushed/master" \
 -			"%(push:remote-name)=to" \
 -			"%(push:remote-ref)=refs/heads/pushed/master"
 +			"%(push:remotename)=to" \
 +			"%(push:remoteref)=refs/heads/pushed/master"
  		do
  			echo "${pair#*=}" >expect &&
  			git for-each-ref --format="${pair%=*}" \
 @@ -701,7 +701,7 @@ test_expect_success ':remote-name and :remote-ref' '
  		git branch push-simple &&
  		git config branch.push-simple.pushRemote from &&
  		actual="$(git for-each-ref \
 -			--format="%(push:remote-name),%(push:remote-ref)" \
 +			--format="%(push:remotename),%(push:remoteref)" \
  			refs/heads/push-simple)" &&
  		test from, = "$actual"
  	)
-- 
2.14.2.windows.1.2.gdc85205db4d

[PATCH v2 1/3] for-each-ref: let upstream/push optionally report the remote name

[Johannes Schindelin]

There are times when e.g. scripts want to know not only the name of the
upstream branch on the remote repository, but also the name of the
remote.

This patch offers the new suffix :remotename for the upstream and for
the push atoms, allowing to show exactly that. Example:

	$ cat .git/config
	...
	[remote "origin"]
		url = https://where.do.we.come/from
		fetch = refs/heads/*:refs/remote/origin/*
	[remote "hello-world"]
		url = https://hello.world/git
		fetch = refs/heads/*:refs/remote/origin/*
		pushURL = hello.world:git
		push = refs/heads/*:refs/heads/*
	[branch "master"]
		remote = origin
		pushRemote = hello-world
	...

	$ git for-each-ref \
		--format='%(upstream) %(upstream:remote) %(push:remote)' \
		refs/heads/master
	refs/remotes/origin/master origin hello-world

The implementation chooses *not* to DWIM the push remote if no explicit
push remote was configured; The reason is that it is possible to DWIM this
by using

	%(if)%(push:remotename)%(then)
		%(push:remotename)
	%(else)
		%(upstream:remotename)
	%(end)

while it would be impossible to "un-DWIM" the information in case the
caller is really only interested in explicit push remotes.

While `:remote` would be shorter, it would also be a bit more ambiguous,
and it would also shut the door e.g. for `:remoteref` (which would
obviously refer to the corresponding ref in the remote repository).

Note: the dashless, non-CamelCased form `:remotename` follows the
example of the `:trackshort` example.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-for-each-ref.txt | 17 ++++++++++-------
 ref-filter.c                       | 35 ++++++++++++++++++++++++++++-------
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 66b4e0a4050..39f50bd53eb 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -140,17 +140,20 @@ upstream::
 	(behind), "<>" (ahead and behind), or "=" (in sync). `:track`
 	also prints "[gone]" whenever unknown upstream ref is
 	encountered. Append `:track,nobracket` to show tracking
-	information without brackets (i.e "ahead N, behind M").  Has
-	no effect if the ref does not have tracking information
-	associated with it.  All the options apart from `nobracket`
-	are mutually exclusive, but if used together the last option
-	is selected.
+	information without brackets (i.e "ahead N, behind M").
++
+Also respects `:remotename` to state the name of the *remote* instead of
+the ref.
++
+Has no effect if the ref does not have tracking information associated
+with it.  All the options apart from `nobracket` are mutually exclusive,
+but if used together the last option is selected.
 
 push::
 	The name of a local ref which represents the `@{push}`
 	location for the displayed ref. Respects `:short`, `:lstrip`,
-	`:rstrip`, `:track`, and `:trackshort` options as `upstream`
-	does. Produces an empty string if no `@{push}` ref is
+	`:rstrip`, `:track`, `:trackshort` and `:remotename` options as
+	`upstream` does. Produces an empty string if no `@{push}` ref is
 	configured.
 
 HEAD::
diff --git a/ref-filter.c b/ref-filter.c
index 55323620ab2..4819707d032 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -76,9 +76,11 @@ static struct used_atom {
 		char color[COLOR_MAXLEN];
 		struct align align;
 		struct {
-			enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+			enum {
+				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
+			} option;
 			struct refname_atom refname;
-			unsigned int nobracket : 1;
+			unsigned int nobracket : 1, push : 1, push_remote : 1;
 		} remote_ref;
 		struct {
 			enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option;
@@ -137,6 +139,9 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 	struct string_list params = STRING_LIST_INIT_DUP;
 	int i;
 
+	if (!strcmp(atom->name, "push") || starts_with(atom->name, "push:"))
+		atom->u.remote_ref.push = 1;
+
 	if (!arg) {
 		atom->u.remote_ref.option = RR_REF;
 		refname_atom_parser_internal(&atom->u.remote_ref.refname,
@@ -156,7 +161,10 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 			atom->u.remote_ref.option = RR_TRACKSHORT;
 		else if (!strcmp(s, "nobracket"))
 			atom->u.remote_ref.nobracket = 1;
-		else {
+		else if (!strcmp(s, "remotename")) {
+			atom->u.remote_ref.option = RR_REMOTE_NAME;
+			atom->u.remote_ref.push_remote = 1;
+		} else {
 			atom->u.remote_ref.option = RR_REF;
 			refname_atom_parser_internal(&atom->u.remote_ref.refname,
 						     arg, atom->name);
@@ -1245,6 +1253,15 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = ">";
 		else
 			*s = "<>";
+	} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {
+		int explicit;
+		const char *remote = atom->u.remote_ref.push ?
+			pushremote_for_branch(branch, &explicit) :
+			remote_for_branch(branch, &explicit);
+		if (explicit)
+			*s = xstrdup(remote);
+		else
+			*s = "";
 	} else
 		die("BUG: unhandled RR_* enum");
 }
@@ -1354,16 +1371,20 @@ static void populate_value(struct ref_array_item *ref)
 			if (refname)
 				fill_remote_ref_details(atom, refname, branch, &v->s);
 			continue;
-		} else if (starts_with(name, "push")) {
+		} else if (atom->u.remote_ref.push) {
 			const char *branch_name;
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name))
 				continue;
 			branch = branch_get(branch_name);
 
-			refname = branch_get_push(branch, NULL);
-			if (!refname)
-				continue;
+			if (atom->u.remote_ref.push_remote)
+				refname = NULL;
+			else {
+				refname = branch_get_push(branch, NULL);
+				if (!refname)
+					continue;
+			}
 			fill_remote_ref_details(atom, refname, branch, &v->s);
 			continue;
 		} else if (starts_with(name, "color:")) {
-- 
2.14.2.windows.1.2.gdc85205db4d

Re: [PATCH v2 1/3] for-each-ref: let upstream/push optionally report the remote name

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> There are times when e.g. scripts want to know not only the name of the
> upstream branch on the remote repository, but also the name of the
> remote.
>
> This patch offers the new suffix :remotename for the upstream and for
> the push atoms, allowing to show exactly that. Example:
> ...
> 	$ git for-each-ref \
> 		--format='%(upstream) %(upstream:remote) %(push:remote)' \
> 		refs/heads/master
> 	refs/remotes/origin/master origin hello-world

While doing the regular re-review of the topics in flight, I noticed
that there are two references to :remote here that should be :remotename,
so I tweaked them locally and pushed the result out as part of 'pu'.

Thanks.

Re: [PATCH v2 1/3] for-each-ref: let upstream/push optionally report the remote name

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The implementation chooses *not* to DWIM the push remote if no explicit
> push remote was configured; The reason is that it is possible to DWIM this
> by using
>
> 	%(if)%(push:remotename)%(then)
> 		%(push:remotename)
> 	%(else)
> 		%(upstream:remotename)
> 	%(end)
>
> while it would be impossible to "un-DWIM" the information in case the
> caller is really only interested in explicit push remotes.

Good.

> Note: the dashless, non-CamelCased form `:remotename` follows the
> example of the `:trackshort` example.

Good.  Come to think of it, aside from modifiers, the use of
squashedwords like committerdate and objecttype has long been the
norm in the for-each-ref atom names, so "remotename" is much more in
line with other formats.

> @@ -1354,16 +1371,20 @@ static void populate_value(struct ref_array_item *ref)
>  			if (refname)
>  				fill_remote_ref_details(atom, refname, branch, &v->s);
>  			continue;
> -		} else if (starts_with(name, "push")) {
> +		} else if (atom->u.remote_ref.push) {
>  			const char *branch_name;
>  			if (!skip_prefix(ref->refname, "refs/heads/",
>  					 &branch_name))
>  				continue;
>  			branch = branch_get(branch_name);
>  
> -			refname = branch_get_push(branch, NULL);
> -			if (!refname)
> -				continue;
> +			if (atom->u.remote_ref.push_remote)
> +				refname = NULL;
> +			else {
> +				refname = branch_get_push(branch, NULL);
> +				if (!refname)
> +					continue;
> +			}
>  			fill_remote_ref_details(atom, refname, branch, &v->s);

It still feels a bit strange to pass NULL refname to
fill_remote_ref_details() function, but fixing it would require
overhaul of this if/else if cascade, I think, so let's not worry too
much about it at least for now.

Looks much better than the previous one.  Thanks.  Queued.

[PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

From: J Wyman <jwyman@microsoft.com>

There are times when scripts want to know not only the name of the
push branch on the remote, but also the name of the branch as known
by the remote repository.

An example of this is when a tool wants to push to the very same branch
from which it would pull automatically, i.e. the `<remote>` and the `<to>`
in `git push <remote> <from>:<to>` would be provided by
`%(upstream:remotename)` and `%(upstream:remoteref)`, respectively.

This patch offers the new suffix :remoteref for the `upstream` and `push`
atoms, allowing to show exactly that. Example:

	$ cat .git/config
	...
	[remote "origin"]
		url = https://where.do.we.come/from
		fetch = refs/heads/*:refs/remote/origin/*
	[branch "master"]
		remote = origin
		merge = refs/heads/master
	[branch "develop/with/topics"]
		remote = origin
		merge = refs/heads/develop/with/topics
	...

	$ git for-each-ref \
		--format='%(push) %(push:remoteref)' \
		refs/heads
	refs/remotes/origin/master refs/heads/master
	refs/remotes/origin/develop/with/topics refs/heads/develop/with/topics

Signed-off-by: J Wyman <jwyman@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-for-each-ref.txt | 11 ++++++-----
 ref-filter.c                       | 13 ++++++++++++-
 remote.c                           | 30 ++++++++++++++++++++++++++++++
 remote.h                           |  2 ++
 4 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 39f50bd53eb..22767025850 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -142,8 +142,9 @@ upstream::
 	encountered. Append `:track,nobracket` to show tracking
 	information without brackets (i.e "ahead N, behind M").
 +
-Also respects `:remotename` to state the name of the *remote* instead of
-the ref.
+Also respects `:remotename` to state the name of the *remote* instead
+of the ref, and `:remoteref` to state the name of the *reference* as
+locally known by the remote.
 +
 Has no effect if the ref does not have tracking information associated
 with it.  All the options apart from `nobracket` are mutually exclusive,
@@ -152,9 +153,9 @@ but if used together the last option is selected.
 push::
 	The name of a local ref which represents the `@{push}`
 	location for the displayed ref. Respects `:short`, `:lstrip`,
-	`:rstrip`, `:track`, `:trackshort` and `:remotename` options as
-	`upstream` does. Produces an empty string if no `@{push}` ref is
-	configured.
+	`:rstrip`, `:track`, `:trackshort`, `:remotename`, and `:remoteref`
+	options as `upstream` does. Produces an empty string if no `@{push}`
+	ref is configured.
 
 HEAD::
 	'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index 4819707d032..b4b2c9b2190 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -77,7 +77,7 @@ static struct used_atom {
 		struct align align;
 		struct {
 			enum {
-				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
+				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME, RR_REMOTE_REF
 			} option;
 			struct refname_atom refname;
 			unsigned int nobracket : 1, push : 1, push_remote : 1;
@@ -164,6 +164,9 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 		else if (!strcmp(s, "remotename")) {
 			atom->u.remote_ref.option = RR_REMOTE_NAME;
 			atom->u.remote_ref.push_remote = 1;
+		} else if (!strcmp(s, "remoteref")) {
+			atom->u.remote_ref.option = RR_REMOTE_REF;
+			atom->u.remote_ref.push_remote = 1;
 		} else {
 			atom->u.remote_ref.option = RR_REF;
 			refname_atom_parser_internal(&atom->u.remote_ref.refname,
@@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = xstrdup(remote);
 		else
 			*s = "";
+	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
+		int explicit, for_push = starts_with(atom->name, "push");
+		const char *merge = remote_ref_for_branch(branch, for_push,
+							  &explicit);
+		if (explicit)
+			*s = xstrdup(merge);
+		else
+			*s = "";
 	} else
 		die("BUG: unhandled RR_* enum");
 }
diff --git a/remote.c b/remote.c
index b220f0dfc61..2bdcfc280cd 100644
--- a/remote.c
+++ b/remote.c
@@ -675,6 +675,36 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
 	return remote_for_branch(branch, explicit);
 }
 
+const char *remote_ref_for_branch(struct branch *branch, int for_push,
+				  int *explicit)
+{
+	if (branch) {
+		if (!for_push) {
+			if (branch->merge_nr) {
+				if (explicit)
+					*explicit = 1;
+				return branch->merge_name[0];
+			}
+		} else {
+			const char *dst, *remote_name =
+				pushremote_for_branch(branch, NULL);
+			struct remote *remote = remote_get(remote_name);
+
+			if (remote && remote->push_refspec_nr &&
+			    (dst = apply_refspecs(remote->push,
+						  remote->push_refspec_nr,
+						  branch->refname))) {
+				if (explicit)
+					*explicit = 1;
+				return dst;
+			}
+		}
+	}
+	if (explicit)
+		*explicit = 0;
+	return "";
+}
+
 static struct remote *remote_get_1(const char *name,
 				   const char *(*get_default)(struct branch *, int *))
 {
diff --git a/remote.h b/remote.h
index 2ecf4c8c74c..1f6611be214 100644
--- a/remote.h
+++ b/remote.h
@@ -223,6 +223,8 @@ struct branch {
 struct branch *branch_get(const char *name);
 const char *remote_for_branch(struct branch *branch, int *explicit);
 const char *pushremote_for_branch(struct branch *branch, int *explicit);
+const char *remote_ref_for_branch(struct branch *branch, int for_push,
+				  int *explicit);
 
 int branch_has_merge_config(struct branch *branch);
 int branch_merge_matches(struct branch *, int n, const char *);
-- 
2.14.2.windows.1.2.gdc85205db4d

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Subject: Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

No verb?  s/optionally/report/ perhaps?

> diff --git a/ref-filter.c b/ref-filter.c
> index 4819707d032..b4b2c9b2190 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -77,7 +77,7 @@ static struct used_atom {
>  		struct align align;
>  		struct {
>  			enum {
> -				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
> +				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME, RR_REMOTE_REF
>  			} option;
>  			struct refname_atom refname;
>  			unsigned int nobracket : 1, push : 1, push_remote : 1;
> @@ -164,6 +164,9 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
>  		else if (!strcmp(s, "remotename")) {
>  			atom->u.remote_ref.option = RR_REMOTE_NAME;
>  			atom->u.remote_ref.push_remote = 1;
> +		} else if (!strcmp(s, "remoteref")) {
> +			atom->u.remote_ref.option = RR_REMOTE_REF;
> +			atom->u.remote_ref.push_remote = 1;
>  		} else {
>  			atom->u.remote_ref.option = RR_REF;
>  			refname_atom_parser_internal(&atom->u.remote_ref.refname,
> @@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
>  			*s = xstrdup(remote);
>  		else
>  			*s = "";
> +	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
> +		int explicit, for_push = starts_with(atom->name, "push");

Hmph, the previous step got rid of starts_with() rather nicely by
introducing atom->u.remote_ref.push bit; can't we do the same in
this step?

Other than that, looks nicer.

Thanks.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
>> Subject: Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name
>
> No verb?  s/optionally/report/ perhaps?

I just noticed that I didn't tweak the copy I have in my tree after
sending this message, but now I did s/optionally/& report the/;

I am still puzzled by the hunk below, though.

>> @@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
>>  			*s = xstrdup(remote);
>>  		else
>>  			*s = "";
>> +	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
>> +		int explicit, for_push = starts_with(atom->name, "push");
>
> Hmph, the previous step got rid of starts_with() rather nicely by
> introducing atom->u.remote_ref.push bit; can't we do the same in
> this step?
>
> Other than that, looks nicer.

Thanks.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Wed, 11 Oct 2017, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >
> >> Subject: Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name
> >
> > No verb?  s/optionally/report/ perhaps?
> 
> I just noticed that I didn't tweak the copy I have in my tree after
> sending this message, but now I did s/optionally/& report the/;

Yes, sorry for not reading this earlier, this is what I meant the commit
subject to say.

> I am still puzzled by the hunk below, though.
> 
> >> @@ -1262,6 +1265,14 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
> >>  			*s = xstrdup(remote);
> >>  		else
> >>  			*s = "";
> >> +	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
> >> +		int explicit, for_push = starts_with(atom->name, "push");
> >
> > Hmph, the previous step got rid of starts_with() rather nicely by
> > introducing atom->u.remote_ref.push bit; can't we do the same in
> > this step?

Right, I forgot to edit this. FWIW I have this in my branch now:

-- snip --
[PATCH] squash! for-each-ref: let upstream/push optionally remote ref
 name

for-each-ref: let upstream/push optionally report the remote ref name

There are times when scripts want to know not only the name of the
push branch on the remote, but also the name of the branch as known
by the remote repository.

An example of this is when a tool wants to push to the very same branch
from which it would pull automatically, i.e. the `<remote>` and the `<to>`
in `git push <remote> <from>:<to>` would be provided by
`%(upstream:remotename)` and `%(upstream:remoteref)`, respectively.

This patch offers the new suffix :remoteref for the `upstream` and `push`
atoms, allowing to show exactly that. Example:

	$ cat .git/config
	...
	[remote "origin"]
		url = https://where.do.we.come/from
		fetch = refs/heads/*:refs/remote/origin/*
	[branch "master"]
		remote = origin
		merge = refs/heads/master
	[branch "develop/with/topics"]
		remote = origin
		merge = refs/heads/develop/with/topics
	...

	$ git for-each-ref \
		--format='%(push) %(push:remoteref)' \
		refs/heads
	refs/remotes/origin/master refs/heads/master
	refs/remotes/origin/develop/with/topics refs/heads/develop/with/topics

Signed-off-by: J Wyman <jwyman@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 ref-filter.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 2556c7885de..6ab12658cb3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1268,9 +1268,11 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 		else
 			*s = "";
 	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
-		int explicit, for_push = starts_with(atom->name, "push");
-		const char *merge = remote_ref_for_branch(branch, for_push,
-							  &explicit);
+		int explicit;
+		const char *merge;
+
+		merge = remote_ref_for_branch(branch, atom->u.remote_ref.push,
+					      &explicit);
 		if (explicit)
 			*s = xstrdup(merge);
 		else
-- 
2.14.2.windows.2
-- snap --

(The funny "squash!" followed by a complete version of the rewritten
commit message is what I do until I -- or anybody else -- comes up with a
patch to implement support for "reword!".)

I'll let this simmer until next week and send out a new iteration of the
patch series then.

Thanks,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> (The funny "squash!" followed by a complete version of the
>> rewritten commit message is what I do until I -- or anybody else
>> -- comes up with a patch to implement support for "reword!".)

I have wished for something like that for a long time; it has been
(and it still is) a bit unclear what the semantics and UI should
look like, but still, such a feature would be quite useful.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Fri, 13 Oct 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> (The funny "squash!" followed by a complete version of the
> >> rewritten commit message is what I do until I -- or anybody else
> >> -- comes up with a patch to implement support for "reword!".)
> 
> I have wished for something like that for a long time; it has been
> (and it still is) a bit unclear what the semantics and UI should
> look like, but still, such a feature would be quite useful.

Oh, the reword! semantics are relatively easy in my mind: it would be
called as

	git commit --reword <commit>

would take staged changes (if any), otherwise implicitly --allow-empty,
then create a commit message to be edited in this style:

	reword! <original-oneline>

	<original-oneline>

	<original-body>

This would be presented to the user in an editor (similar to `git commit
--amend`).

Upon rebase --autosquash, it would work like a squash! but comment out all
previous commit messages, and also comment out the reword! line and the
empty line after that.

In case of multiple reword!, the last one would win, and it would
naturally integrate in any fixup!/squash! workflow.

What is *more* difficult is something else I frequently need: drop!
<oneline>. That is, I want to explicitly mark a commit to be excluded in
subsequent rebase --autosquash. I *guess* the best way would be to
implement a

	git revert --drop <commit>

that would work as if you called `git revert -n <commit> && git commit -m
'drop! '"$(git show -s --oneline <commit>)", and upon rebase --autosquash,
it would reorder like fixup!/squash!/reword!, replace the `pick` of the
previous line (if it was a `pick`) by `drop` and comment out the current
`pick drop! <oneline>` line.

The reason why the semantics are more difficult in that case is that drop!
does not mix well with fixup!/squash!/reword!.

Ciao,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Kevin Daudt]

On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:
> From: J Wyman <jwyman@microsoft.com>
> [..] 
> 
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 39f50bd53eb..22767025850 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -142,8 +142,9 @@ upstream::
>  	encountered. Append `:track,nobracket` to show tracking
>  	information without brackets (i.e "ahead N, behind M").
>  +
> -Also respects `:remotename` to state the name of the *remote* instead of
> -the ref.
> +Also respects `:remotename` to state the name of the *remote* instead
> +of the ref, and `:remoteref` to state the name of the *reference* as
> +locally known by the remote.

What does "locally known by the remote" mean in this sentence?

>  +
>  Has no effect if the ref does not have tracking information associated
>  with it.  All the options apart from `nobracket` are mutually exclusive,
> @@ -152,9 +153,9 @@ but if used together the last option is selected.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:
>> From: J Wyman <jwyman@microsoft.com>
>> [..] 
>> 
>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>> index 39f50bd53eb..22767025850 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -142,8 +142,9 @@ upstream::
>>  	encountered. Append `:track,nobracket` to show tracking
>>  	information without brackets (i.e "ahead N, behind M").
>>  +
>> -Also respects `:remotename` to state the name of the *remote* instead of
>> -the ref.
>> +Also respects `:remotename` to state the name of the *remote* instead
>> +of the ref, and `:remoteref` to state the name of the *reference* as
>> +locally known by the remote.
>
> What does "locally known by the remote" mean in this sentence?

Good question.  I cannot offhand offer a better and concise
phrasing, but I think can explain what it wants to describe ;-).

For a local branch we have (say, 'topic'), there may be different
things outside this repository that regularly interact with it.

We may update 'topic' with work others did, by

	git checkout topic && git pull

without any extra command line argument to "git pull".  We are
pulling from the 'upstream' of the 'topic', which is a branch in a
remote repository, and the (nick)name of the remote is :remotename.
When we do this pull, we would grab one of the branches the remote
has and merge it into our 'topic'.  IOW, when the above command line
is written in longhand, the "git pull" step may look like this [*1*]:

	git pull origin devel

if we were building on top of the 'devel' branch at the 'origin'
remote.  The full refname of that branch, 'refs/heads/devel', is
:remoteref, and that is the reference locally known to the 'origin'
remote.

The "locally known by the remote" is an attempt by the patch authors
to stress the fact that the result is not refs/remotes/origin/devel
(which is where _you_ would keep a local copy of that reference in
this repository).

Two other things outside this repository that interact with 'topic'
are where the result of our work is pushed out to, which is a branch
at the 'push' remote.


[Footnotes]

*1* Modulo the details of other refs fetched that are unrelated to
    the resulting merge and local copies stored as remote-tracking
    branches in this repository.

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi,

On Sat, 14 Oct 2017, Junio C Hamano wrote:

> Kevin Daudt <me@ikke.info> writes:
> 
> > On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:
> >> From: J Wyman <jwyman@microsoft.com>
> >> [..] 
> >> 
> >> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> >> index 39f50bd53eb..22767025850 100644
> >> --- a/Documentation/git-for-each-ref.txt
> >> +++ b/Documentation/git-for-each-ref.txt
> >> @@ -142,8 +142,9 @@ upstream::
> >>  	encountered. Append `:track,nobracket` to show tracking
> >>  	information without brackets (i.e "ahead N, behind M").
> >>  +
> >> -Also respects `:remotename` to state the name of the *remote* instead of
> >> -the ref.
> >> +Also respects `:remotename` to state the name of the *remote* instead
> >> +of the ref, and `:remoteref` to state the name of the *reference* as
> >> +locally known by the remote.
> >
> > What does "locally known by the remote" mean in this sentence?
> 
> Good question.  I cannot offhand offer a better and concise
> phrasing, but I think can explain what it wants to describe ;-).

Yep, described it well.

Maybe "and `:remoteref` to state the name by which the remote knows the
*reference*"?

Ciao,
Dscho

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> >> -Also respects `:remotename` to state the name of the *remote* instead of
>> >> -the ref.
>> >> +Also respects `:remotename` to state the name of the *remote* instead
>> >> +of the ref, and `:remoteref` to state the name of the *reference* as
>> >> +locally known by the remote.
>> >
>> > What does "locally known by the remote" mean in this sentence?
>> 
>> Good question.  I cannot offhand offer a better and concise
>> phrasing, but I think can explain what it wants to describe ;-).
>
> Yep, described it well.
>
> Maybe "and `:remoteref` to state the name by which the remote knows the
> *reference*"?

I dunno.  

The original and your update both seem to come from a worldview
where there is a single conceptual thing called "reference" that is
shared between our repository and the remote repository we pull from
(or push to), and the name we call it is "refs/remotes/origin/devel"
while the name they use to call it is "refs/heads/devel".  If you
subscribe to that worldview, then the updated sentence might make it
clearer than the original.

But I do not share that worldview, and I am not sure (note: I am
truly unsure---it's not like I am convinced it is a good idea but
sugarcoating my expression, at least in this case) if subscribing to
the worldview helps users' understanding.

In my view "refs/remotes/origin/devel" is a reference we use to keep
track of (or "a reference that corresponds to") the reference they
have called "refs/heads/devel" at the remote, and these are two
separate entities, so it's not like "this single thing is called
differently by us and them".

Stepping back a bit; here is how the description begins.

    upstream::
            The name of a local ref which can be considered ``upstream''
            from the displayed ref.

So 'upstream' is like 'refs/remotes/origin/devel' in the example in
the message you are responding to.  Perhaps we can make it clear in
the description, and then add :remote* modifiers are about asking
where that remote-tracking branch comes from.  For example, instead
of these "Also respects...", something like:

    For a %(upstream) that is a remote-tracking branch, the name of
    the remote repository it is copied from can be obtained with
    %(upstream:remotename).  Simiarly, the branch at the remote
    repository whose tip is copioed to this remote-tracking branch
    can be obtined with %(upstream:remoteref) as a full refname.

may reduce the chance of confusion?

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Mon, 16 Oct 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> >> -Also respects `:remotename` to state the name of the *remote* instead of
> >> >> -the ref.
> >> >> +Also respects `:remotename` to state the name of the *remote* instead
> >> >> +of the ref, and `:remoteref` to state the name of the *reference* as
> >> >> +locally known by the remote.
> >> >
> >> > What does "locally known by the remote" mean in this sentence?
> >> 
> >> Good question.  I cannot offhand offer a better and concise
> >> phrasing, but I think can explain what it wants to describe ;-).
> >
> > Yep, described it well.
> >
> > Maybe "and `:remoteref` to state the name by which the remote knows the
> > *reference*"?
> 
> I dunno.  
> 
> The original and your update both seem to come from a worldview
> where there is a single conceptual thing called "reference" that is
> shared between our repository and the remote repository we pull from
> (or push to), and the name we call it is "refs/remotes/origin/devel"
> while the name they use to call it is "refs/heads/devel".  If you
> subscribe to that worldview, then the updated sentence might make it
> clearer than the original.
> 
> But I do not share that worldview, and I am not sure (note: I am
> truly unsure---it's not like I am convinced it is a good idea but
> sugarcoating my expression, at least in this case) if subscribing to
> the worldview helps users' understanding.
> 
> In my view "refs/remotes/origin/devel" is a reference we use to keep
> track of (or "a reference that corresponds to") the reference they
> have called "refs/heads/devel" at the remote, and these are two
> separate entities, so it's not like "this single thing is called
> differently by us and them".
> 
> Stepping back a bit; here is how the description begins.
> 
>     upstream::
>             The name of a local ref which can be considered ``upstream''
>             from the displayed ref.
> 
> So 'upstream' is like 'refs/remotes/origin/devel' in the example in
> the message you are responding to.  Perhaps we can make it clear in
> the description, and then add :remote* modifiers are about asking
> where that remote-tracking branch comes from.  For example, instead
> of these "Also respects...", something like:
> 
>     For a %(upstream) that is a remote-tracking branch, the name of
>     the remote repository it is copied from can be obtained with
>     %(upstream:remotename).  Simiarly, the branch at the remote
>     repository whose tip is copioed to this remote-tracking branch
>     can be obtined with %(upstream:remoteref) as a full refname.
> 
> may reduce the chance of confusion?

Let's take two more steps back.

First, for-each-ref is a low-level command (I do not remember whether our
nickname for "low-level" is "plumbing" or "porcelain" or anything, so I
stick with "low-level" which I deem descriptive enough). That is, users of
this command (and therefore, readers of this man page) need to be quite
familiar with Git's worldview.

Second, the main purpose of this patch series is to answer the question
"what is the default <remoteref> in `git push <remotename>
<ref>:<remoteref>`?" for *many* refs at once.

Maybe it would be better to describe the functionality by describing the
question it tries to answer.

Ciao,
Dscho

[PATCH v2 3/3] for-each-ref: test :remotename and :remoteref

[Johannes Schindelin]

This not only prevents regressions, but also serves as documentation
what this new feature is expected to do.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6300-for-each-ref.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2274a4b7331..d9eb2be0256 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -675,4 +675,36 @@ test_expect_success 'Verify usage of %(symref:rstrip) atom' '
 	test_cmp expected actual
 '
 
+test_expect_success ':remotename and :remoteref' '
+	git init remote-tests &&
+	(
+		cd remote-tests &&
+		test_commit initial &&
+		git remote add from fifth.coffee:blub &&
+		git config branch.master.remote from &&
+		git config branch.master.merge refs/heads/stable &&
+		git remote add to southridge.audio:repo &&
+		git config remote.to.push "refs/heads/*:refs/heads/pushed/*" &&
+		git config branch.master.pushRemote to &&
+		for pair in "%(upstream)=refs/remotes/from/stable" \
+			"%(upstream:remotename)=from" \
+			"%(upstream:remoteref)=refs/heads/stable" \
+			"%(push)=refs/remotes/to/pushed/master" \
+			"%(push:remotename)=to" \
+			"%(push:remoteref)=refs/heads/pushed/master"
+		do
+			echo "${pair#*=}" >expect &&
+			git for-each-ref --format="${pair%=*}" \
+				refs/heads/master >actual &&
+			test_cmp expect actual
+		done &&
+		git branch push-simple &&
+		git config branch.push-simple.pushRemote from &&
+		actual="$(git for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from, = "$actual"
+	)
+'
+
 test_done
-- 
2.14.2.windows.1.2.gdc85205db4d

[PATCH v3 0/3] for-each-ref: add :remoteref and :remotename specifiers

[Johannes Schindelin]

This introduces support for

	git for-each-ref \
		--format="%(merge:remotename),%(merge:remoteref)"
	git for-each-ref \
		--format="%(push:remotename),%(push:remoteref)"

to figure out the remote nickname as well as the name of the corresponding
branch on the remote.

Note: the `%(push:remotename)` placeholder is only interpolated by the value
of `branch.<name>.pushRemote`; unlike `git push`, it does not fall back to
`branch.<name>.remote`. Likewise, `%(push:remoteref)` interpolates to the
empty string unless `remote.<nick>.pushRefs` is configured.

This is useful for third-party tools that need to know this type of information
for tons of branches.

Changes since v2:

- fixed the commit message of 1/3 to no longer talk about :remote.

- used the push atom in 2/3, made the code look more as suggested by Junio.

- fixed the oneline of 2/3 to use a verb ("report").

- butchered Junio's proposed documentation update for 2/3 to hopefully make
  the description of :remotename and :remoteref a lot clearer.


J Wyman (1):
  for-each-ref: let upstream/push report the remote ref name

Johannes Schindelin (2):
  for-each-ref: let upstream/push optionally report the remote name
  for-each-ref: test :remotename and :remoteref

 Documentation/git-for-each-ref.txt | 23 +++++++++++-------
 ref-filter.c                       | 48 ++++++++++++++++++++++++++++++++------
 remote.c                           | 30 ++++++++++++++++++++++++
 remote.h                           |  2 ++
 t/t6300-for-each-ref.sh            | 32 +++++++++++++++++++++++++
 5 files changed, 120 insertions(+), 15 deletions(-)


base-commit: 7668cbc60578f99a4c048f8f8f38787930b8147b
Published-As: https://github.com/dscho/git/releases/tag/ref-filter-remote-name-v3
Fetch-It-Via: git fetch https://github.com/dscho/git ref-filter-remote-name-v3

Interdiff vs v2:
 diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
 index aff44c064a4..dffa14a7950 100644
 --- a/Documentation/git-for-each-ref.txt
 +++ b/Documentation/git-for-each-ref.txt
 @@ -147,9 +147,12 @@ upstream::
  	encountered. Append `:track,nobracket` to show tracking
  	information without brackets (i.e "ahead N, behind M").
  +
 -Also respects `:remotename` to state the name of the *remote* instead
 -of the ref, and `:remoteref` to state the name of the *reference* as
 -locally known by the remote.
 +For any remote-tracking branch `%(upstream)`, `%(upstream:remotename)`
 +and `%(upstream:remoteref)` refer to the name of the remote and the
 +name of the tracked remote ref, respectively. In other words, the
 +remote-tracking branch can be updated explicitly and individually by
 +using the refspec `%(upstream:remoteref):%(upstream)` to fetch from
 +`%(upstream:remotename)`.
  +
  Has no effect if the ref does not have tracking information associated
  with it.  All the options apart from `nobracket` are mutually exclusive,
 diff --git a/ref-filter.c b/ref-filter.c
 index bf078657d91..3f9161707e6 100644
 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -1289,9 +1289,11 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
  		else
  			*s = "";
  	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
 -		int explicit, for_push = starts_with(atom->name, "push");
 -		const char *merge = remote_ref_for_branch(branch, for_push,
 -							  &explicit);
 +		int explicit;
 +		const char *merge;
 +
 +		merge = remote_ref_for_branch(branch, atom->u.remote_ref.push,
 +					      &explicit);
  		if (explicit)
  			*s = xstrdup(merge);
  		else
-- 
2.15.0.windows.1

[PATCH v3 1/3] for-each-ref: let upstream/push optionally report the remote name

[Johannes Schindelin]

There are times when e.g. scripts want to know not only the name of the
upstream branch on the remote repository, but also the name of the
remote.

This patch offers the new suffix :remotename for the upstream and for
the push atoms, allowing to show exactly that. Example:

	$ cat .git/config
	...
	[remote "origin"]
		url = https://where.do.we.come/from
		fetch = refs/heads/*:refs/remote/origin/*
	[remote "hello-world"]
		url = https://hello.world/git
		fetch = refs/heads/*:refs/remote/origin/*
		pushURL = hello.world:git
		push = refs/heads/*:refs/heads/*
	[branch "master"]
		remote = origin
		pushRemote = hello-world
	...

	$ git for-each-ref \
	  --format='%(upstream) %(upstream:remotename) %(push:remotename)' \
	  refs/heads/master
	refs/remotes/origin/master origin hello-world

The implementation chooses *not* to DWIM the push remote if no explicit
push remote was configured; The reason is that it is possible to DWIM this
by using

	%(if)%(push:remotename)%(then)
		%(push:remotename)
	%(else)
		%(upstream:remotename)
	%(end)

while it would be impossible to "un-DWIM" the information in case the
caller is really only interested in explicit push remotes.

While `:remote` would be shorter, it would also be a bit more ambiguous,
and it would also shut the door e.g. for `:remoteref` (which would
obviously refer to the corresponding ref in the remote repository).

Note: the dashless, non-CamelCased form `:remotename` follows the
example of the `:trackshort` example.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-for-each-ref.txt | 17 ++++++++++-------
 ref-filter.c                       | 35 ++++++++++++++++++++++++++++-------
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 1d420e4cde8..288b11aaeed 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -145,17 +145,20 @@ upstream::
 	(behind), "<>" (ahead and behind), or "=" (in sync). `:track`
 	also prints "[gone]" whenever unknown upstream ref is
 	encountered. Append `:track,nobracket` to show tracking
-	information without brackets (i.e "ahead N, behind M").  Has
-	no effect if the ref does not have tracking information
-	associated with it.  All the options apart from `nobracket`
-	are mutually exclusive, but if used together the last option
-	is selected.
+	information without brackets (i.e "ahead N, behind M").
++
+Also respects `:remotename` to state the name of the *remote* instead of
+the ref.
++
+Has no effect if the ref does not have tracking information associated
+with it.  All the options apart from `nobracket` are mutually exclusive,
+but if used together the last option is selected.
 
 push::
 	The name of a local ref which represents the `@{push}`
 	location for the displayed ref. Respects `:short`, `:lstrip`,
-	`:rstrip`, `:track`, and `:trackshort` options as `upstream`
-	does. Produces an empty string if no `@{push}` ref is
+	`:rstrip`, `:track`, `:trackshort` and `:remotename` options as
+	`upstream` does. Produces an empty string if no `@{push}` ref is
 	configured.
 
 HEAD::
diff --git a/ref-filter.c b/ref-filter.c
index e728b15b3ae..cde8b97dcb0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -76,9 +76,11 @@ static struct used_atom {
 		char color[COLOR_MAXLEN];
 		struct align align;
 		struct {
-			enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+			enum {
+				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
+			} option;
 			struct refname_atom refname;
-			unsigned int nobracket : 1;
+			unsigned int nobracket : 1, push : 1, push_remote : 1;
 		} remote_ref;
 		struct {
 			enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option;
@@ -138,6 +140,9 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 	struct string_list params = STRING_LIST_INIT_DUP;
 	int i;
 
+	if (!strcmp(atom->name, "push") || starts_with(atom->name, "push:"))
+		atom->u.remote_ref.push = 1;
+
 	if (!arg) {
 		atom->u.remote_ref.option = RR_REF;
 		refname_atom_parser_internal(&atom->u.remote_ref.refname,
@@ -157,7 +162,10 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 			atom->u.remote_ref.option = RR_TRACKSHORT;
 		else if (!strcmp(s, "nobracket"))
 			atom->u.remote_ref.nobracket = 1;
-		else {
+		else if (!strcmp(s, "remotename")) {
+			atom->u.remote_ref.option = RR_REMOTE_NAME;
+			atom->u.remote_ref.push_remote = 1;
+		} else {
 			atom->u.remote_ref.option = RR_REF;
 			refname_atom_parser_internal(&atom->u.remote_ref.refname,
 						     arg, atom->name);
@@ -1268,6 +1276,15 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = ">";
 		else
 			*s = "<>";
+	} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {
+		int explicit;
+		const char *remote = atom->u.remote_ref.push ?
+			pushremote_for_branch(branch, &explicit) :
+			remote_for_branch(branch, &explicit);
+		if (explicit)
+			*s = xstrdup(remote);
+		else
+			*s = "";
 	} else
 		die("BUG: unhandled RR_* enum");
 }
@@ -1377,16 +1394,20 @@ static void populate_value(struct ref_array_item *ref)
 			if (refname)
 				fill_remote_ref_details(atom, refname, branch, &v->s);
 			continue;
-		} else if (starts_with(name, "push")) {
+		} else if (atom->u.remote_ref.push) {
 			const char *branch_name;
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name))
 				continue;
 			branch = branch_get(branch_name);
 
-			refname = branch_get_push(branch, NULL);
-			if (!refname)
-				continue;
+			if (atom->u.remote_ref.push_remote)
+				refname = NULL;
+			else {
+				refname = branch_get_push(branch, NULL);
+				if (!refname)
+					continue;
+			}
 			fill_remote_ref_details(atom, refname, branch, &v->s);
 			continue;
 		} else if (starts_with(name, "color:")) {
-- 
2.15.0.windows.1

[PATCH v3 2/3] for-each-ref: let upstream/push report the remote ref name

[Johannes Schindelin]

From: J Wyman <jwyman@microsoft.com>

There are times when scripts want to know not only the name of the
push branch on the remote, but also the name of the branch as known
by the remote repository.

An example of this is when a tool wants to push to the very same branch
from which it would pull automatically, i.e. the `<remote>` and the `<to>`
in `git push <remote> <from>:<to>` would be provided by
`%(upstream:remotename)` and `%(upstream:remoteref)`, respectively.

This patch offers the new suffix :remoteref for the `upstream` and `push`
atoms, allowing to show exactly that. Example:

	$ cat .git/config
	...
	[remote "origin"]
		url = https://where.do.we.come/from
		fetch = refs/heads/*:refs/remote/origin/*
	[branch "master"]
		remote = origin
		merge = refs/heads/master
	[branch "develop/with/topics"]
		remote = origin
		merge = refs/heads/develop/with/topics
	...

	$ git for-each-ref \
		--format='%(push) %(push:remoteref)' \
		refs/heads
	refs/remotes/origin/master refs/heads/master
	refs/remotes/origin/develop/with/topics refs/heads/develop/with/topics

Signed-off-by: J Wyman <jwyman@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-for-each-ref.txt | 14 +++++++++-----
 ref-filter.c                       | 15 ++++++++++++++-
 remote.c                           | 30 ++++++++++++++++++++++++++++++
 remote.h                           |  2 ++
 4 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 288b11aaeed..dffa14a7950 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -147,8 +147,12 @@ upstream::
 	encountered. Append `:track,nobracket` to show tracking
 	information without brackets (i.e "ahead N, behind M").
 +
-Also respects `:remotename` to state the name of the *remote* instead of
-the ref.
+For any remote-tracking branch `%(upstream)`, `%(upstream:remotename)`
+and `%(upstream:remoteref)` refer to the name of the remote and the
+name of the tracked remote ref, respectively. In other words, the
+remote-tracking branch can be updated explicitly and individually by
+using the refspec `%(upstream:remoteref):%(upstream)` to fetch from
+`%(upstream:remotename)`.
 +
 Has no effect if the ref does not have tracking information associated
 with it.  All the options apart from `nobracket` are mutually exclusive,
@@ -157,9 +161,9 @@ but if used together the last option is selected.
 push::
 	The name of a local ref which represents the `@{push}`
 	location for the displayed ref. Respects `:short`, `:lstrip`,
-	`:rstrip`, `:track`, `:trackshort` and `:remotename` options as
-	`upstream` does. Produces an empty string if no `@{push}` ref is
-	configured.
+	`:rstrip`, `:track`, `:trackshort`, `:remotename`, and `:remoteref`
+	options as `upstream` does. Produces an empty string if no `@{push}`
+	ref is configured.
 
 HEAD::
 	'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index cde8b97dcb0..3f9161707e6 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -77,7 +77,7 @@ static struct used_atom {
 		struct align align;
 		struct {
 			enum {
-				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME
+				RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME, RR_REMOTE_REF
 			} option;
 			struct refname_atom refname;
 			unsigned int nobracket : 1, push : 1, push_remote : 1;
@@ -165,6 +165,9 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_
 		else if (!strcmp(s, "remotename")) {
 			atom->u.remote_ref.option = RR_REMOTE_NAME;
 			atom->u.remote_ref.push_remote = 1;
+		} else if (!strcmp(s, "remoteref")) {
+			atom->u.remote_ref.option = RR_REMOTE_REF;
+			atom->u.remote_ref.push_remote = 1;
 		} else {
 			atom->u.remote_ref.option = RR_REF;
 			refname_atom_parser_internal(&atom->u.remote_ref.refname,
@@ -1285,6 +1288,16 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = xstrdup(remote);
 		else
 			*s = "";
+	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
+		int explicit;
+		const char *merge;
+
+		merge = remote_ref_for_branch(branch, atom->u.remote_ref.push,
+					      &explicit);
+		if (explicit)
+			*s = xstrdup(merge);
+		else
+			*s = "";
 	} else
 		die("BUG: unhandled RR_* enum");
 }
diff --git a/remote.c b/remote.c
index 685e776a65e..4e93753e198 100644
--- a/remote.c
+++ b/remote.c
@@ -675,6 +675,36 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
 	return remote_for_branch(branch, explicit);
 }
 
+const char *remote_ref_for_branch(struct branch *branch, int for_push,
+				  int *explicit)
+{
+	if (branch) {
+		if (!for_push) {
+			if (branch->merge_nr) {
+				if (explicit)
+					*explicit = 1;
+				return branch->merge_name[0];
+			}
+		} else {
+			const char *dst, *remote_name =
+				pushremote_for_branch(branch, NULL);
+			struct remote *remote = remote_get(remote_name);
+
+			if (remote && remote->push_refspec_nr &&
+			    (dst = apply_refspecs(remote->push,
+						  remote->push_refspec_nr,
+						  branch->refname))) {
+				if (explicit)
+					*explicit = 1;
+				return dst;
+			}
+		}
+	}
+	if (explicit)
+		*explicit = 0;
+	return "";
+}
+
 static struct remote *remote_get_1(const char *name,
 				   const char *(*get_default)(struct branch *, int *))
 {
diff --git a/remote.h b/remote.h
index 2ecf4c8c74c..1f6611be214 100644
--- a/remote.h
+++ b/remote.h
@@ -223,6 +223,8 @@ struct branch {
 struct branch *branch_get(const char *name);
 const char *remote_for_branch(struct branch *branch, int *explicit);
 const char *pushremote_for_branch(struct branch *branch, int *explicit);
+const char *remote_ref_for_branch(struct branch *branch, int for_push,
+				  int *explicit);
 
 int branch_has_merge_config(struct branch *branch);
 int branch_merge_matches(struct branch *, int n, const char *);
-- 
2.15.0.windows.1

[PATCH v3 3/3] for-each-ref: test :remotename and :remoteref

[Johannes Schindelin]

This not only prevents regressions, but also serves as documentation
what this new feature is expected to do.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6300-for-each-ref.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 3aa534933e0..c128dfc5790 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -766,4 +766,36 @@ test_expect_success 'Verify usage of %(symref:rstrip) atom' '
 	test_cmp expected actual
 '
 
+test_expect_success ':remotename and :remoteref' '
+	git init remote-tests &&
+	(
+		cd remote-tests &&
+		test_commit initial &&
+		git remote add from fifth.coffee:blub &&
+		git config branch.master.remote from &&
+		git config branch.master.merge refs/heads/stable &&
+		git remote add to southridge.audio:repo &&
+		git config remote.to.push "refs/heads/*:refs/heads/pushed/*" &&
+		git config branch.master.pushRemote to &&
+		for pair in "%(upstream)=refs/remotes/from/stable" \
+			"%(upstream:remotename)=from" \
+			"%(upstream:remoteref)=refs/heads/stable" \
+			"%(push)=refs/remotes/to/pushed/master" \
+			"%(push:remotename)=to" \
+			"%(push:remoteref)=refs/heads/pushed/master"
+		do
+			echo "${pair#*=}" >expect &&
+			git for-each-ref --format="${pair%=*}" \
+				refs/heads/master >actual &&
+			test_cmp expect actual
+		done &&
+		git branch push-simple &&
+		git config branch.push-simple.pushRemote from &&
+		actual="$(git for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from, = "$actual"
+	)
+'
+
 test_done
-- 
2.15.0.windows.1

Re: [PATCH v3 0/3] for-each-ref: add :remoteref and :remotename specifiers

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Changes since v2:
>
> - fixed the commit message of 1/3 to no longer talk about :remote.

OK.  It now matches what we have had since October 5th in my tree.

> - used the push atom in 2/3, made the code look more as suggested by Junio.

I am pleasantly surprised (mostly because I didn't realize that the
code has already prepared all the info necessary to do this in the
atom enumeration phase) that this change was quite easy and doably
very cleanly.

> - fixed the oneline of 2/3 to use a verb ("report").
>
> - butchered Junio's proposed documentation update for 2/3 to hopefully make
>   the description of :remotename and :remoteref a lot clearer.

Well I wasn't attempting to propose anything---as I said, I couldn't
concisely phrase what the description in v2 wanted to say in clearer
terms, so I gave a lengthy and fuller description.

It feels a bit out-of-place to see "can be updated" in the
description of a read-only operation for-each-ref, as we see below:

>  -Also respects `:remotename` to state the name of the *remote* instead
>  -of the ref, and `:remoteref` to state the name of the *reference* as
>  -locally known by the remote.
>  +For any remote-tracking branch `%(upstream)`, `%(upstream:remotename)`
>  +and `%(upstream:remoteref)` refer to the name of the remote and the
>  +name of the tracked remote ref, respectively. In other words, the
>  +remote-tracking branch can be updated explicitly and individually by
>  +using the refspec `%(upstream:remoteref):%(upstream)` to fetch from
>  +`%(upstream:remotename)`.

but I think this should do for now.  It certainly is much clearer
than the previous round.

Will queue, and merge to 'next' soonish.

Thanks.