patch, 3 partsfor-each-ref: add :remote-ref and :remote-name specifiers

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

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

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

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

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

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.

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

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?

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

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.

Hi Junio,

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

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

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

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

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.

Changes to this file in this patch looks sane.

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.

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.

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.

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

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

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.

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

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.

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(+)

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

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.

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

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

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

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.

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

Good.

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.

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.

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

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

No verb?  s/optionally/report/ perhaps?

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.

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

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.

Thanks.

Hi Junio,

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

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

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

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

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.

Hi Junio,

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

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

On Thu, Oct 05, 2017 at 02:19:15PM +0200, Johannes Schindelin wrote:

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

Kevin Daudt <me@ikke.info> writes:

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.

Hi,

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

Yep, described it well.

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

Ciao, Dscho

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

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?

Hi Junio,

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

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

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(+)

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

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

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

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(+)

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

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

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.

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:

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.