Re: [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any.

[Manuel Jacob <me@manueljacob.de>]

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