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

classic Classic list List threaded Threaded
26 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
Karthik Nayak <[hidden email]> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Karthik Nayak
On Tue, May 17, 2016 at 11:22 PM, Junio C Hamano <[hidden email]> wrote:

> Karthik Nayak <[hidden email]> 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?

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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v6 05/17] ref-filter: move get_head_description() from branch.c

Junio C Hamano
In reply to this post by Karthik Nayak
Karthik Nayak <[hidden email]> writes:

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 2ecde53..141168d 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -361,39 +361,6 @@ static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
>   strbuf_release(&subject);
>  }
>  
> -static char *get_head_description(void)
> -{
> - struct strbuf desc = STRBUF_INIT;
> - struct wt_status_state state;
> - memset(&state, 0, sizeof(state));
> - wt_status_get_state(&state, 1);
> - if (state.rebase_in_progress ||
> -    state.rebase_interactive_in_progress)
> - strbuf_addf(&desc, _("(no branch, rebasing %s)"),
> -    state.branch);
> - else if (state.bisect_in_progress)
> - strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
> -    state.branch);
> - else if (state.detached_from) {
> - 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);
> - }

Note that this expects that va/i18n-misc-updates topic, which
corrects the translator instruction around here, is already applied.

> diff --git a/ref-filter.c b/ref-filter.c
> index 7d3af1c..fcb3353 100644
> ...
> +char *get_head_description(void)
> +{
> + struct strbuf desc = STRBUF_INIT;
> + struct wt_status_state state;
> + memset(&state, 0, sizeof(state));
> + wt_status_get_state(&state, 1);
> + if (state.rebase_in_progress ||
> +    state.rebase_interactive_in_progress)
> + strbuf_addf(&desc, _("(no branch, rebasing %s)"),
> +    state.branch);
> + else if (state.bisect_in_progress)
> + 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)
> + strbuf_addf(&desc, _("(HEAD detached at %s)"),
> + state.detached_from);
> + else
> + strbuf_addf(&desc, _("(HEAD detached from %s)"),
> + state.detached_from);
> + }

... but the change is apparently lost.

It is a good lesson not to blindly rebase things on 'next', which
would have unrelated changes.  If you needed es/test-gpg-tags topic
for the test script change, check out 'master', merge that single
topic, and then rebase the series on top of the result.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
In reply to this post by Karthik Nayak
Karthik Nayak <[hidden email]> 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
 '

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Karthik Nayak
On Wed, May 18, 2016 at 2:00 AM, Junio C Hamano <[hidden email]> wrote:

> Karthik Nayak <[hidden email]> 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
>  '
>

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

--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v6 05/17] ref-filter: move get_head_description() from branch.c

Karthik Nayak
In reply to this post by Junio C Hamano
>
> Note that this expects that va/i18n-misc-updates topic, which
> corrects the translator instruction around here, is already applied.
>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 7d3af1c..fcb3353 100644
>> ...
>> +char *get_head_description(void)
>> +{
>> +     struct strbuf desc = STRBUF_INIT;
>> +     struct wt_status_state state;
>> +     memset(&state, 0, sizeof(state));
>> +     wt_status_get_state(&state, 1);
>> +     if (state.rebase_in_progress ||
>> +         state.rebase_interactive_in_progress)
>> +             strbuf_addf(&desc, _("(no branch, rebasing %s)"),
>> +                         state.branch);
>> +     else if (state.bisect_in_progress)
>> +             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)
>> +                     strbuf_addf(&desc, _("(HEAD detached at %s)"),
>> +                             state.detached_from);
>> +             else
>> +                     strbuf_addf(&desc, _("(HEAD detached from %s)"),
>> +                             state.detached_from);
>> +     }
>
> ... but the change is apparently lost.
>
> It is a good lesson not to blindly rebase things on 'next', which
> would have unrelated changes.  If you needed es/test-gpg-tags topic
> for the test script change, check out 'master', merge that single
> topic, and then rebase the series on top of the result.
>

Lesson learned. Will do that from now on :)

--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
12