Re: [PATCH v6 00/17] Port branch.c to use ref-filter's printing options

Karthik Nayak [off-list ref] writes:

Hello, sorry for the confusion, it's built on top of 'next' which contains
f307218 (t6302: simplify non-gpg cases). The merge conflict is due to the
commit made by you 1cca17df (Documentation: fix linkgit references).

That is not "confusion", but an "incorrect piece of information".

The series does not seem to apply on 'next', either.

Where did you exactly rebase on top of?  It is not on f307218, it is
not on 'next', 'next@{1}',... 'next@{8}'.

f3072180 (t6302: simplify non-gpg cases, 2016-05-09) was merged to
'next' at 9fcb98b2 (Merge branch 'es/test-gpg-tags' into next,
2016-05-10), but the series does not seem to apply there, either.

$ git co 9fcb98b2
Applying: ref-filter: implement %(if), %(then), and %(else) atoms
error: patch failed: Documentation/git-for-each-ref.txt:181
error: Documentation/git-for-each-ref.txt: patch does not apply
Patch failed at 0001 ref-filter: implement %(if), %(then), and %(else) atoms
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Not that a series built on top of any 'next' is directly usable.
You are forcing me to identify which topics in 'next' you depend on,
and build a topic that does not contain anything unrelated that is
in 'next' before starting to apply these patches.  Can you pick a
more appropriate place to base these patches on, please?  Why isn't
this based on 'master', for example?

On Tue, May 17, 2016 at 11:22 PM, Junio C Hamano [off-list ref] wrote:

Karthik Nayak [off-list ref] writes:

quoted

Hello, sorry for the confusion, it's built on top of 'next' which contains
f307218 (t6302: simplify non-gpg cases). The merge conflict is due to the
commit made by you 1cca17df (Documentation: fix linkgit references).

That is not "confusion", but an "incorrect piece of information".

The series does not seem to apply on 'next', either.

Where did you exactly rebase on top of?  It is not on f307218, it is
not on 'next', 'next@{1}',... 'next@{8}'.

f3072180 (t6302: simplify non-gpg cases, 2016-05-09) was merged to
'next' at 9fcb98b2 (Merge branch 'es/test-gpg-tags' into next,
2016-05-10), but the series does not seem to apply there, either.

$ git co 9fcb98b2
Applying: ref-filter: implement %(if), %(then), and %(else) atoms
error: patch failed: Documentation/git-for-each-ref.txt:181
error: Documentation/git-for-each-ref.txt: patch does not apply
Patch failed at 0001 ref-filter: implement %(if), %(then), and %(else) atoms
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Not that a series built on top of any 'next' is directly usable.
You are forcing me to identify which topics in 'next' you depend on,
and build a topic that does not contain anything unrelated that is
in 'next' before starting to apply these patches.  Can you pick a
more appropriate place to base these patches on, please?  Why isn't
this based on 'master', for example?

Hello,

Sorry for that.
The only reason I haven't based it on 'master' is because it doesn't contain
'f307218'.

➔ git branch --contains=f307218
  next
  ref-filter

Now speaking of which, this is based on next.

➔ git branch -v
    * next       78b384c Sync with master

And Idk what the problem is but it seems to apply perfectly on top of it [1]

➔ git am v6-00*
Applying: ref-filter: implement %(if), %(then), and %(else) atoms
Applying: ref-filter: include reference to 'used_atom' within 'atom_value'
Applying: ref-filter: implement %(if:equals=<string>) and
%(if:notequals=<string>)
Applying: ref-filter: modify "%(objectname:short)" to take length
Applying: ref-filter: move get_head_description() from branch.c
Applying: ref-filter: introduce format_ref_array_item()
Applying: ref-filter: make %(upstream:track) prints "[gone]" for
invalid upstreams
Applying: ref-filter: add support for %(upstream:track,nobracket)
Applying: ref-filter: make "%(symref)" atom work with the ':short' modifier
Applying: ref-filter: introduce refname_atom_parser_internal()
Applying: ref-filter: introduce symref_atom_parser() and refname_atom_parser()
Applying: ref-filter: make remote_ref_atom_parser() use
refname_atom_parser_internal()
Applying: ref-filter: add `:dir` and `:base` options for ref printing atoms
Applying: ref-filter: allow porcelain to translate messages in the output
Applying: branch, tag: use porcelain output
Applying: branch: use ref-filter printing APIs
Applying: branch: implement '--format' option

[1] : https://github.com/KarthikNayak/git/commits/ref-filter

-- 
Regards,
Karthik Nayak

Karthik Nayak [off-list ref] writes:

Sorry for that.
The only reason I haven't based it on 'master' is because it doesn't contain
'f307218'.

➔ git branch --contains=f307218
  next
  ref-filter

It is not clear from the above what your local ref-filter contains
beyond 'master', so it is not very useful to me when I am trying to
help you to avoid taking this topic hostage to all the topics in
'next' (if I queued this directly on 'next', I have to hold this
series until all the topics in 'next' graduates to 'master).

The series certainly would not apply to f307218 at all; it depends
on other stuff you have either in your local 'ref-filter' or 'next'.
It does not apply to the result of a merge of 'es/test-gpg-tags' topic 
into 'master', either, but the above does not make it clear what
else you are using from 'next' at all.

In any case, I think I managed to reduce the dependency on only
'es/test-gpg-tags' and 'master', and that is what I'll be queuing.
Please double check the patches in kn/ref-filter-branch-list topic
and also the merge of it into 'pu' for mismerges.

The difference between the result of merging the previously queued
one to 'pu', and the result of merging this round to 'pu', looks
like the attached.

Thanks.

 builtin/branch.c               |  2 +-
 ref-filter.c                   |  6 ++++--
 t/t6302-for-each-ref-filter.sh | 30 +++++++++++++++---------------
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0bbb4de..2412738 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -293,7 +293,7 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
 		skip_prefix(it->refname, "refs/remotes/", &desc);
 		if (it->kind == FILTER_REFS_DETACHED_HEAD) {
 			char *head_desc = get_head_description();
-			w = strlen(head_desc);
+			w = utf8_strwidth(head_desc);
 			free(head_desc);
 		} else
 			w = utf8_strwidth(desc);

diff --git a/ref-filter.c b/ref-filter.c
index 74c4869..f25671c 100644
--- a/ref-filter.c
+++ b/ref-filter.c

@@ -1196,12 +1196,14 @@ char *get_head_description(void)
 		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
 			    state.branch);
 	else if (state.detached_from) {
-		/* TRANSLATORS: make sure these match _("HEAD detached at ")
-		   and _("HEAD detached from ") in wt-status.c */
 		if (state.detached_at)
+			/* TRANSLATORS: make sure this matches
+			   "HEAD detached at " in wt-status.c */
 			strbuf_addf(&desc, _("(HEAD detached at %s)"),
 				state.detached_from);
 		else
+			/* TRANSLATORS: make sure this matches
+			   "HEAD detached from " in wt-status.c */
 			strbuf_addf(&desc, _("(HEAD detached from %s)"),
 				state.detached_from);
 	}

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 331d978..a09a1a4 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh

@@ -342,22 +342,22 @@ test_expect_success 'improper usage of %(if), %(then), %(else) and %(end) atoms'
 '
 
 test_expect_success 'check %(if)...%(then)...%(end) atoms' '
-	git for-each-ref --format="%(if)%(authorname)%(then)%(authorname): %(refname)%(end)" >actual &&
+	git for-each-ref --format="%(refname)%(if)%(authorname)%(then) Author: %(authorname)%(end)" >actual &&
 	cat >expect <<-\EOF &&
-	A U Thor: refs/heads/master
-	A U Thor: refs/heads/side
-	A U Thor: refs/odd/spot
-
-
-
-	A U Thor: refs/tags/foo1.10
-	A U Thor: refs/tags/foo1.3
-	A U Thor: refs/tags/foo1.6
-	A U Thor: refs/tags/four
-	A U Thor: refs/tags/one
-
-	A U Thor: refs/tags/three
-	A U Thor: refs/tags/two
+	refs/heads/master Author: A U Thor
+	refs/heads/side Author: A U Thor
+	refs/odd/spot Author: A U Thor
+	refs/tags/annotated-tag
+	refs/tags/doubly-annotated-tag
+	refs/tags/doubly-signed-tag
+	refs/tags/foo1.10 Author: A U Thor
+	refs/tags/foo1.3 Author: A U Thor
+	refs/tags/foo1.6 Author: A U Thor
+	refs/tags/four Author: A U Thor
+	refs/tags/one Author: A U Thor
+	refs/tags/signed-tag
+	refs/tags/three Author: A U Thor
+	refs/tags/two Author: A U Thor
 	EOF
 	test_cmp expect actual
 '

On Wed, May 18, 2016 at 2:00 AM, Junio C Hamano [off-list ref] wrote:

Seems to be right, Thanks for putting in the extra effort, Will ensure
this doesn't
happen again from my side.

-- 
Regards,
Karthik Nayak