From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 32111 invoked from network); 21 Jun 2020 22:23:47 -0000 Received: from ns1.primenet.com.au (HELO primenet.com.au) (203.24.36.2) by inbox.vuxu.org with ESMTPUTF8; 21 Jun 2020 22:23:47 -0000 Received: (qmail 2999 invoked by alias); 21 Jun 2020 22:23:22 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: Sender: zsh-workers@zsh.org X-Seq: 46092 Received: (qmail 14061 invoked by uid 1010); 21 Jun 2020 22:23:22 -0000 X-Qmail-Scanner-Diagnostics: from indus.uberspace.de by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.3/25850. spamassassin: 3.4.4. Clear:RC:0(95.143.172.201):SA:0(-1.9/5.0):. Processed in 2.416709 secs); 21 Jun 2020 22:23:22 -0000 X-Envelope-From: me@manueljacob.de X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at manueljacob.de does not designate permitted sender hosts) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Mon, 22 Jun 2020 00:22:45 +0200 From: Manuel Jacob To: Daniel Shahaf Cc: zsh-workers@zsh.org Subject: Re: [PATCH v3] Add code to Mercurial VCS backend to show topic if there is any. In-Reply-To: <20200621122546.4272f43f@tarpaulin.shahaf.local2> References: <20200619233112.191102-1-me@manueljacob.de> <20200620104317.51bac64a@tarpaulin.shahaf.local2> <20200621122546.4272f43f@tarpaulin.shahaf.local2> Message-ID: <6331a4a9664315f291361e26d169a8cd@manueljacob.de> X-Sender: 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 «$(> > 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