zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any.
@ 2020-06-19 23:31 Manuel Jacob
  2020-06-20 10:43 ` Daniel Shahaf
  0 siblings, 1 reply; 5+ messages in thread
From: Manuel Jacob @ 2020-06-19 23:31 UTC (permalink / raw)
  To: zsh-workers; +Cc: Manuel Jacob

"Topics" is an experimental concept in Mercurial that augments the
current branching concept (called "named branches").

For more information, see the not always up-to-date Mercurial Wiki page
https://www.mercurial-scm.org/wiki/TopicPlan.
---
 Functions/VCS_Info/Backends/VCS_INFO_get_data_hg | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
index cd5ef321d..deeb331a0 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
@@ -5,7 +5,7 @@
 
 setopt localoptions extendedglob NO_shwordsplit
 
-local hgbase bmfile branchfile rebasefile dirstatefile mqseriesfile \
+local hgbase bmfile branchfile topicfile rebasefile dirstatefile mqseriesfile \
     curbmfile curbm \
     mqstatusfile mqguardsfile patchdir mergedir \
     r_csetid r_lrev r_branch i_bmhash i_bmname \
@@ -27,6 +27,7 @@ mergedir="${hgbase}/.hg/merge/"
 bmfile="${hgbase}/.hg/bookmarks"
 curbmfile="${hgbase}/.hg/bookmarks.current"
 branchfile="${hgbase}/.hg/branch"
+topicfile="${hgbase}/.hg/topic"
 rebasefile="${hgbase}/.hg/rebasestate"
 dirstatefile="${hgbase}/.hg/dirstate"
 mqstatusfile="${patchdir}/status" # currently applied patches
@@ -69,6 +70,13 @@ fi
 # If we still don't know the branch it's safe to assume default
 [[ -n ${r_branch} ]] || r_branch="default"
 
+# Show topic if there is any (the UI for this experimental concept is not yet
+# final, but for a long time the convention has been to join the branch name
+# and the topic name by a colon)
+if [[ -f ${topicfile} && -r ${topicfile} && -s ${topicfile} ]] ; then
+    r_branch=${r_branch}:$(head -n 1 ${topicfile})
+fi
+
 # The working dir has uncommitted-changes if the revision ends with a +
 if [[ $r_lrev[-1] == + ]] ; then
     hgchanges=1
-- 
2.27.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-19 23:31 [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
@ 2020-06-20 10:43 ` Daniel Shahaf
  2020-06-21 11:17   ` Manuel Jacob
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Shahaf @ 2020-06-20 10:43 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

Manuel Jacob wrote on Sat, 20 Jun 2020 01:31 +0200:
> "Topics" is an experimental concept in Mercurial that augments the
> current branching concept (called "named branches").
> 
> For more information, see the not always up-to-date Mercurial Wiki page
> https://www.mercurial-scm.org/wiki/TopicPlan.
> ---
>  Functions/VCS_Info/Backends/VCS_INFO_get_data_hg | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
> index cd5ef321d..deeb331a0 100644
> --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
> +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
> @@ -5,7 +5,7 @@
>  
>  setopt localoptions extendedglob NO_shwordsplit
>  
> -local hgbase bmfile branchfile rebasefile dirstatefile mqseriesfile \
> +local hgbase bmfile branchfile topicfile rebasefile dirstatefile mqseriesfile \
>      curbmfile curbm \
>      mqstatusfile mqguardsfile patchdir mergedir \
>      r_csetid r_lrev r_branch i_bmhash i_bmname \
> @@ -27,6 +27,7 @@ mergedir="${hgbase}/.hg/merge/"
>  bmfile="${hgbase}/.hg/bookmarks"
>  curbmfile="${hgbase}/.hg/bookmarks.current"
>  branchfile="${hgbase}/.hg/branch"
> +topicfile="${hgbase}/.hg/topic"
>  rebasefile="${hgbase}/.hg/rebasestate"
>  dirstatefile="${hgbase}/.hg/dirstate"
>  mqstatusfile="${patchdir}/status" # currently applied patches
> @@ -69,6 +70,13 @@ fi
>  # If we still don't know the branch it's safe to assume default
>  [[ -n ${r_branch} ]] || r_branch="default"
>  
> +# Show topic if there is any (the UI for this experimental concept is not yet
> +# final, but for a long time the convention has been to join the branch name
> +# and the topic name by a colon)
> +if [[ -f ${topicfile} && -r ${topicfile} && -s ${topicfile} ]] ; then
> +    r_branch=${r_branch}:$(head -n 1 ${topicfile})
> +fi

I'm happy to read the file directly, despite experimentality, given the
discussion elsethread that you'll propose a corresponding unit test to
the upstream maintainers.

Using $(head) forks, and we try to avoid forks because they're expensive
on some platforms.  In this case, you could use «IFS= read -r REPLY
<$topicfile» instead.  (The construct «$(<foo)», just above the hunk
context, is special-cased to not fork.)

As written, existing set-branch-format hooks would see the ${hook_com[branch]}
input argument set to strings such as "default:mytopic".  Would this
behaviour be expected, or would people rather expect ${hook_com[branch]}
to be set to strings such as "default" even when .hg/topic exists, and
to have a new ${hook_com[foo]} key whose value would be "mytopic" (or
possibly "default:mytopic")?  This would affect the API to hooks but
would not affect the final output.

Speaking of which, have you tested the output with the get-revision
style set?  (Run «zstyle ':vcs_info:hg:*' get-revision true»
interactively or in your zshrc.)  As written, I expect this patch would
result in output such as «default:mytopic:c04503e1dba4:0» with the style
set, compared to «default:c04503e1dba4:0» with the style set in current
master.  I don't see a problem here; just raising the point for
awareness.

(The latter output can be customised by setting the the hgrevformat and
branchformat styles with the 'zstyle' builtin.  In the examplar output
above, "c04503e1dba4" is %r in hgrevformat; the final "0" is %h in
hgrevformat; "c04503e1dba4:0", as an atomic unit, is %r in branchformat;
and "default"/"default:mytopic" is %b in branchformat.  The whole
shebang is then %b in the formats/actionformats styles.)

Similarly, as written, the %b expando in "formats", "actionformats", and
"branchformat" will always be replaced with a string such as
"default:mytopic".  If someone wants to change the format,¹ they won't
be able to achieve that using zstyle settings alone; they'd have to
write an appropriate vcs_info hook in their zshrc.  What do you think
of, say, making the topic name available as a separate expando in
branchformat?  Again, this would not affect the default output, but make
customizations easier to implement.

¹ Use-cases for changing the format include: colouring "default" and
"mytopic" differently (compare «zstyle '*' branchformat '%B%b%%b %U%r%u'»
when the get-revision style is set); changing the colon to a space for
readability or copypasteability; and perhaps even eliding the topic
name entirely(?).

>  # The working dir has uncommitted-changes if the revision ends with a +
>  if [[ $r_lrev[-1] == + ]] ; then
>      hgchanges=1

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-20 10:43 ` Daniel Shahaf
@ 2020-06-21 11:17   ` Manuel Jacob
  2020-06-21 12:25     ` Daniel Shahaf
  0 siblings, 1 reply; 5+ messages in thread
From: Manuel Jacob @ 2020-06-21 11:17 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 2020-06-20 12:43, Daniel Shahaf wrote:
> Manuel Jacob wrote on Sat, 20 Jun 2020 01:31 +0200:
>> "Topics" is an experimental concept in Mercurial that augments the
>> current branching concept (called "named branches").
>> 
>> For more information, see the not always up-to-date Mercurial Wiki 
>> page
>> https://www.mercurial-scm.org/wiki/TopicPlan.
>> ---
>>  Functions/VCS_Info/Backends/VCS_INFO_get_data_hg | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg 
>> b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
>> index cd5ef321d..deeb331a0 100644
>> --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
>> +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
>> @@ -5,7 +5,7 @@
>> 
>>  setopt localoptions extendedglob NO_shwordsplit
>> 
>> -local hgbase bmfile branchfile rebasefile dirstatefile mqseriesfile \
>> +local hgbase bmfile branchfile topicfile rebasefile dirstatefile 
>> mqseriesfile \
>>      curbmfile curbm \
>>      mqstatusfile mqguardsfile patchdir mergedir \
>>      r_csetid r_lrev r_branch i_bmhash i_bmname \
>> @@ -27,6 +27,7 @@ mergedir="${hgbase}/.hg/merge/"
>>  bmfile="${hgbase}/.hg/bookmarks"
>>  curbmfile="${hgbase}/.hg/bookmarks.current"
>>  branchfile="${hgbase}/.hg/branch"
>> +topicfile="${hgbase}/.hg/topic"
>>  rebasefile="${hgbase}/.hg/rebasestate"
>>  dirstatefile="${hgbase}/.hg/dirstate"
>>  mqstatusfile="${patchdir}/status" # currently applied patches
>> @@ -69,6 +70,13 @@ fi
>>  # If we still don't know the branch it's safe to assume default
>>  [[ -n ${r_branch} ]] || r_branch="default"
>> 
>> +# Show topic if there is any (the UI for this experimental concept is 
>> not yet
>> +# final, but for a long time the convention has been to join the 
>> branch name
>> +# and the topic name by a colon)
>> +if [[ -f ${topicfile} && -r ${topicfile} && -s ${topicfile} ]] ; then
>> +    r_branch=${r_branch}:$(head -n 1 ${topicfile})
>> +fi
> 
> I'm happy to read the file directly, despite experimentality, given the
> discussion elsethread that you'll propose a corresponding unit test to
> the upstream maintainers.
> 
> Using $(head) forks, and we try to avoid forks because they're 
> expensive
> on some platforms.  In this case, you could use «IFS= read -r REPLY
> <$topicfile» instead.  (The construct «$(<foo)», just above the hunk
> context, is special-cased to not fork.)

Is there anything special about `REPLY`? It seems to be a convention. 
Could I also use e.g. TOPIC?

> As written, existing set-branch-format hooks would see the 
> ${hook_com[branch]}
> input argument set to strings such as "default:mytopic".  Would this
> behaviour be expected, or would people rather expect 
> ${hook_com[branch]}
> to be set to strings such as "default" even when .hg/topic exists, and
> to have a new ${hook_com[foo]} key whose value would be "mytopic" (or
> possibly "default:mytopic")?  This would affect the API to hooks but
> would not affect the final output.

If the topic extension is enabled, the meaning of places using branch 
names is changed.
When determining how many heads a branch has (e.g. when warning that 
another head will be created), each "branch:topic" (or simply "branch" 
if there’s no topic) is considered separately.
When specifying a revision, "branch:topic" refers to the head of the 
branch with branch name "branch" and topic name "topic", while "branch" 
refers to the headmost revision with branch name "branch" but no topic.

An example:

B C
|/
A

Assume that all three changesets have the "branch" field set to 
"develop". B has topic "feature1" and C has topic "feature2".

* "develop" refers to A
* "develop:feature1" refers to B
* "develop:feature2" refers to C

If the user updated to B, but the prompt shows "develop" (this is 
currently the case), it would be misleading. I don’t know all 
possibilities in which the hooks use `${hook_com[branch]}`, but I’d 
expect that including the topic would be correct in more cases than not 
including it.

> Speaking of which, have you tested the output with the get-revision
> style set?  (Run «zstyle ':vcs_info:hg:*' get-revision true»
> interactively or in your zshrc.)  As written, I expect this patch would
> result in output such as «default:mytopic:c04503e1dba4:0» with the 
> style
> set, compared to «default:c04503e1dba4:0» with the style set in current
> master.  I don't see a problem here; just raising the point for
> awareness.

It overloads ":" to mean both "delimits traditional branch name and 
topic name" and "delimits branch name (in the general sense) and further 
information". I don’t see a problem. Users should be able to distinguish 
the different fields.

> (The latter output can be customised by setting the the hgrevformat and
> branchformat styles with the 'zstyle' builtin.  In the examplar output
> above, "c04503e1dba4" is %r in hgrevformat; the final "0" is %h in
> hgrevformat; "c04503e1dba4:0", as an atomic unit, is %r in 
> branchformat;
> and "default"/"default:mytopic" is %b in branchformat.  The whole
> shebang is then %b in the formats/actionformats styles.)
> 
> Similarly, as written, the %b expando in "formats", "actionformats", 
> and
> "branchformat" will always be replaced with a string such as
> "default:mytopic".  If someone wants to change the format,¹ they won't
> be able to achieve that using zstyle settings alone; they'd have to
> write an appropriate vcs_info hook in their zshrc.  What do you think
> of, say, making the topic name available as a separate expando in
> branchformat?  Again, this would not affect the default output, but 
> make
> customizations easier to implement.
> 
> ¹ Use-cases for changing the format include: colouring "default" and
> "mytopic" differently (compare «zstyle '*' branchformat '%B%b%%b 
> %U%r%u'»
> when the get-revision style is set); changing the colon to a space for
> readability or copypasteability; and perhaps even eliding the topic
> name entirely(?).

For the reasons stated above, the topic name should be included. The 
colon should also stay because the combined "branch:topic" can be passed 
as a revision identifier to various places. Coloring branch and topic 
differently wouldn’t be "wrong", but since the "branch:topic" mostly 
makes sense together, personally I’d like to have it in the same 
appearance.

>>  # The working dir has uncommitted-changes if the revision ends with a 
>> +
>>  if [[ $r_lrev[-1] == + ]] ; then
>>      hgchanges=1
> 
> Cheers,
> 
> Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-21 11:17   ` Manuel Jacob
@ 2020-06-21 12:25     ` Daniel Shahaf
  2020-06-21 22:22       ` Manuel Jacob
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Shahaf @ 2020-06-21 12:25 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

Manuel Jacob wrote on Sun, 21 Jun 2020 13:17 +0200:
> On 2020-06-20 12:43, Daniel Shahaf wrote:
> > Manuel Jacob wrote on Sat, 20 Jun 2020 01:31 +0200:
> >> "Topics" is an experimental concept in Mercurial that augments the
> >> current branching concept (called "named branches").
> >> 
> >> For more information, see the not always up-to-date Mercurial Wiki 
> >> page
> >> https://www.mercurial-scm.org/wiki/TopicPlan.
> >> ---
> >>  Functions/VCS_Info/Backends/VCS_INFO_get_data_hg | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg 
> >> b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
> >> index cd5ef321d..deeb331a0 100644
> >> --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
> >> +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
> >> @@ -5,7 +5,7 @@
> >> 
> >>  setopt localoptions extendedglob NO_shwordsplit
> >> 
> >> -local hgbase bmfile branchfile rebasefile dirstatefile mqseriesfile \
> >> +local hgbase bmfile branchfile topicfile rebasefile dirstatefile 
> >> mqseriesfile \
> >>      curbmfile curbm \
> >>      mqstatusfile mqguardsfile patchdir mergedir \
> >>      r_csetid r_lrev r_branch i_bmhash i_bmname \
> >> @@ -27,6 +27,7 @@ mergedir="${hgbase}/.hg/merge/"
> >>  bmfile="${hgbase}/.hg/bookmarks"
> >>  curbmfile="${hgbase}/.hg/bookmarks.current"
> >>  branchfile="${hgbase}/.hg/branch"
> >> +topicfile="${hgbase}/.hg/topic"
> >>  rebasefile="${hgbase}/.hg/rebasestate"
> >>  dirstatefile="${hgbase}/.hg/dirstate"
> >>  mqstatusfile="${patchdir}/status" # currently applied patches
> >> @@ -69,6 +70,13 @@ fi
> >>  # If we still don't know the branch it's safe to assume default
> >>  [[ -n ${r_branch} ]] || r_branch="default"
> >> 
> >> +# Show topic if there is any (the UI for this experimental concept is 
> >> not yet
> >> +# final, but for a long time the convention has been to join the 
> >> branch name
> >> +# and the topic name by a colon)
> >> +if [[ -f ${topicfile} && -r ${topicfile} && -s ${topicfile} ]] ; then
> >> +    r_branch=${r_branch}:$(head -n 1 ${topicfile})
> >> +fi
> > 
> > I'm happy to read the file directly, despite experimentality, given the
> > discussion elsethread that you'll propose a corresponding unit test to
> > the upstream maintainers.
> > 
> > Using $(head) forks, and we try to avoid forks because they're 
> > expensive
> > on some platforms.  In this case, you could use «IFS= read -r REPLY
> > <$topicfile» instead.  (The construct «$(<foo)», just above the hunk
> > context, is special-cased to not fork.)
> 
> Is there anything special about `REPLY`? It seems to be a convention. 
> Could I also use e.g. TOPIC?

Yes, $REPLY is merely a convention (see zshparam(1)).  You could use
$topic (lowercase) instead; $TOPIC (uppercase) would not be idiomatic.

> > As written, existing set-branch-format hooks would see the 
> > ${hook_com[branch]}
> > input argument set to strings such as "default:mytopic".  Would this
> > behaviour be expected, or would people rather expect 
> > ${hook_com[branch]}
> > to be set to strings such as "default" even when .hg/topic exists, and
> > to have a new ${hook_com[foo]} key whose value would be "mytopic" (or
> > possibly "default:mytopic")?  This would affect the API to hooks but
> > would not affect the final output.
> 
> If the topic extension is enabled, the meaning of places using branch 
> names is changed.
> When determining how many heads a branch has (e.g. when warning that 
> another head will be created), each "branch:topic" (or simply "branch" 
> if there’s no topic) is considered separately.
> When specifying a revision, "branch:topic" refers to the head of the 
> branch with branch name "branch" and topic name "topic", while "branch" 
> refers to the headmost revision with branch name "branch" but no topic.
> 
> An example:
> 
> B C
> |/
> A
> 
> Assume that all three changesets have the "branch" field set to 
> "develop". B has topic "feature1" and C has topic "feature2".
> 
> * "develop" refers to A
> * "develop:feature1" refers to B
> * "develop:feature2" refers to C
> 
> If the user updated to B, but the prompt shows "develop" (this is 
> currently the case), it would be misleading. I don’t know all 
> possibilities in which the hooks use `${hook_com[branch]}`, but I’d 
> expect that including the topic would be correct in more cases than not 
> including it.
> 

Thanks for the walkthrough.  Agreed.  Might want to state on
Doc/Zsh/contrib.yo:1331 that in the hg backend the topic is included,
then.

> > Similarly, as written, the %b expando in "formats", "actionformats", 
> > and
> > "branchformat" will always be replaced with a string such as
> > "default:mytopic".  If someone wants to change the format,¹ they won't
> > be able to achieve that using zstyle settings alone; they'd have to
> > write an appropriate vcs_info hook in their zshrc.  What do you think
> > of, say, making the topic name available as a separate expando in
> > branchformat?  Again, this would not affect the default output, but 
> > make
> > customizations easier to implement.
> > 
> > ¹ Use-cases for changing the format include: colouring "default" and
> > "mytopic" differently (compare «zstyle '*' branchformat '%B%b%%b 
> > %U%r%u'»
> > when the get-revision style is set); changing the colon to a space for
> > readability or copypasteability; and perhaps even eliding the topic
> > name entirely(?).
> 
> For the reasons stated above, the topic name should be included. The 
> colon should also stay because the combined "branch:topic" can be passed 
> as a revision identifier to various places. Coloring branch and topic 
> differently wouldn’t be "wrong", but since the "branch:topic" mostly 
> makes sense together, personally I’d like to have it in the same 
> appearance.

I see: if "branch:topic" mostly appears as an atomic unit, it makes
sense that most people will want to color it atomically.

>

That's all, I think.  I don't foresee any blocks to merging v4 when you
post it.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-21 12:25     ` Daniel Shahaf
@ 2020-06-21 22:22       ` Manuel Jacob
  0 siblings, 0 replies; 5+ messages in thread
From: Manuel Jacob @ 2020-06-21 22:22 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 2020-06-21 14:25, Daniel Shahaf wrote:
> Manuel Jacob wrote on Sun, 21 Jun 2020 13:17 +0200:
>> On 2020-06-20 12:43, Daniel Shahaf wrote:
>> > Manuel Jacob wrote on Sat, 20 Jun 2020 01:31 +0200:
>> >> "Topics" is an experimental concept in Mercurial that augments the
>> >> current branching concept (called "named branches").
>> >>
>> >> For more information, see the not always up-to-date Mercurial Wiki
>> >> page
>> >> https://www.mercurial-scm.org/wiki/TopicPlan.
>> >> ---
>> >>  Functions/VCS_Info/Backends/VCS_INFO_get_data_hg | 10 +++++++++-
>> >>  1 file changed, 9 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
>> >> b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
>> >> index cd5ef321d..deeb331a0 100644
>> >> --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
>> >> +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
>> >> @@ -5,7 +5,7 @@
>> >>
>> >>  setopt localoptions extendedglob NO_shwordsplit
>> >>
>> >> -local hgbase bmfile branchfile rebasefile dirstatefile mqseriesfile \
>> >> +local hgbase bmfile branchfile topicfile rebasefile dirstatefile
>> >> mqseriesfile \
>> >>      curbmfile curbm \
>> >>      mqstatusfile mqguardsfile patchdir mergedir \
>> >>      r_csetid r_lrev r_branch i_bmhash i_bmname \
>> >> @@ -27,6 +27,7 @@ mergedir="${hgbase}/.hg/merge/"
>> >>  bmfile="${hgbase}/.hg/bookmarks"
>> >>  curbmfile="${hgbase}/.hg/bookmarks.current"
>> >>  branchfile="${hgbase}/.hg/branch"
>> >> +topicfile="${hgbase}/.hg/topic"
>> >>  rebasefile="${hgbase}/.hg/rebasestate"
>> >>  dirstatefile="${hgbase}/.hg/dirstate"
>> >>  mqstatusfile="${patchdir}/status" # currently applied patches
>> >> @@ -69,6 +70,13 @@ fi
>> >>  # If we still don't know the branch it's safe to assume default
>> >>  [[ -n ${r_branch} ]] || r_branch="default"
>> >>
>> >> +# Show topic if there is any (the UI for this experimental concept is
>> >> not yet
>> >> +# final, but for a long time the convention has been to join the
>> >> branch name
>> >> +# and the topic name by a colon)
>> >> +if [[ -f ${topicfile} && -r ${topicfile} && -s ${topicfile} ]] ; then
>> >> +    r_branch=${r_branch}:$(head -n 1 ${topicfile})
>> >> +fi
>> >
>> > I'm happy to read the file directly, despite experimentality, given the
>> > discussion elsethread that you'll propose a corresponding unit test to
>> > the upstream maintainers.
>> >
>> > Using $(head) forks, and we try to avoid forks because they're
>> > expensive
>> > on some platforms.  In this case, you could use «IFS= read -r REPLY
>> > <$topicfile» instead.  (The construct «$(<foo)», just above the hunk
>> > context, is special-cased to not fork.)
>> 
>> Is there anything special about `REPLY`? It seems to be a convention.
>> Could I also use e.g. TOPIC?
> 
> Yes, $REPLY is merely a convention (see zshparam(1)).  You could use
> $topic (lowercase) instead; $TOPIC (uppercase) would not be idiomatic.
> 
>> > As written, existing set-branch-format hooks would see the
>> > ${hook_com[branch]}
>> > input argument set to strings such as "default:mytopic".  Would this
>> > behaviour be expected, or would people rather expect
>> > ${hook_com[branch]}
>> > to be set to strings such as "default" even when .hg/topic exists, and
>> > to have a new ${hook_com[foo]} key whose value would be "mytopic" (or
>> > possibly "default:mytopic")?  This would affect the API to hooks but
>> > would not affect the final output.
>> 
>> If the topic extension is enabled, the meaning of places using branch
>> names is changed.
>> When determining how many heads a branch has (e.g. when warning that
>> another head will be created), each "branch:topic" (or simply "branch"
>> if there’s no topic) is considered separately.
>> When specifying a revision, "branch:topic" refers to the head of the
>> branch with branch name "branch" and topic name "topic", while 
>> "branch"
>> refers to the headmost revision with branch name "branch" but no 
>> topic.
>> 
>> An example:
>> 
>> B C
>> |/
>> A
>> 
>> Assume that all three changesets have the "branch" field set to
>> "develop". B has topic "feature1" and C has topic "feature2".
>> 
>> * "develop" refers to A
>> * "develop:feature1" refers to B
>> * "develop:feature2" refers to C
>> 
>> If the user updated to B, but the prompt shows "develop" (this is
>> currently the case), it would be misleading. I don’t know all
>> possibilities in which the hooks use `${hook_com[branch]}`, but I’d
>> expect that including the topic would be correct in more cases than 
>> not
>> including it.
>> 
> 
> Thanks for the walkthrough.  Agreed.  Might want to state on
> Doc/Zsh/contrib.yo:1331 that in the hg backend the topic is included,
> then.

Since the topic name is hg-specific but the line in the documentation 
applies to all backends, I’ll keep it short (I’ll spare details about 
the exact format).

>> > Similarly, as written, the %b expando in "formats", "actionformats",
>> > and
>> > "branchformat" will always be replaced with a string such as
>> > "default:mytopic".  If someone wants to change the format,¹ they won't
>> > be able to achieve that using zstyle settings alone; they'd have to
>> > write an appropriate vcs_info hook in their zshrc.  What do you think
>> > of, say, making the topic name available as a separate expando in
>> > branchformat?  Again, this would not affect the default output, but
>> > make
>> > customizations easier to implement.
>> >
>> > ¹ Use-cases for changing the format include: colouring "default" and
>> > "mytopic" differently (compare «zstyle '*' branchformat '%B%b%%b
>> > %U%r%u'»
>> > when the get-revision style is set); changing the colon to a space for
>> > readability or copypasteability; and perhaps even eliding the topic
>> > name entirely(?).
>> 
>> For the reasons stated above, the topic name should be included. The
>> colon should also stay because the combined "branch:topic" can be 
>> passed
>> as a revision identifier to various places. Coloring branch and topic
>> differently wouldn’t be "wrong", but since the "branch:topic" mostly
>> makes sense together, personally I’d like to have it in the same
>> appearance.
> 
> I see: if "branch:topic" mostly appears as an atomic unit, it makes
> sense that most people will want to color it atomically.
> 
>> 
> 
> That's all, I think.  I don't foresee any blocks to merging v4 when you
> post it.
> 
> Cheers,
> 
> Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-21 22:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-19 23:31 [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
2020-06-20 10:43 ` Daniel Shahaf
2020-06-21 11:17   ` Manuel Jacob
2020-06-21 12:25     ` Daniel Shahaf
2020-06-21 22:22       ` Manuel Jacob

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).