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

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Manuel Jacob wrote on Sat, 20 Jun 2020 01:29 +0200:
> On 2020-06-19 12:07, Daniel Shahaf wrote:
> > Manuel Jacob wrote on Fri, 19 Jun 2020 04:06 +0200:  
> >> On 2020-06-17 10:27, Daniel Shahaf wrote:  
> >> > Daniel Shahaf wrote on Tue, 09 Jun 2020 22:34 +0000:  
> >> >> Manuel Jacob wrote on Tue, 09 Jun 2020 03:15 +0200:  
> >> >> > On 2020-06-08 07:31, Daniel Shahaf wrote:  
> >> >> > > Manuel Jacob wrote on Sun, 07 Jun 2020 15:31 +0200:  
> >> >> > >> Hi Daniel,
> >> >> > >>
> >> >> > >> On 2020-06-07 14:14, Daniel Shahaf wrote:  
> >> >> > >> > Manuel Jacob wrote on Sun, 07 Jun 2020 09:44 +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.  
> >> >> > >> >
> >> >> > >> > I assume "experimental" means "future releases of Mercurial are not
> >> >> > >> > promised to be backwards compatible with the current design".  
> >> >> > >>
> >> >> > >> It is not yet part of Mercurial itself, but developed separately as an
> >> >> > >> extension under the Mercurial project roof.
> >> >> > >>  
> >> >> > >
> >> >> > > Thanks for the info.  In practice, though, it doesn't make that much
> >> >> > > difference to vcs_info whether the functionality lives in Mercurial
> >> >> > > core
> >> >> > > or in an extension developed by the same development team.  I was more
> >> >> > > concerned with whether the current on-disk data format is promised to
> >> >> > > be
> >> >> > > supported by future releases of the functionality [whether those be in
> >> >> > > hg core or in extensions].
> >> >> > >  
> >> >> > >> > Is the .hg/topic file name and data format set in stone yet?
> >> >> > >> >
> >> >> > >> > What if zsh 5.9 is released and then the Mercurial developers change
> >> >> > >> > the design to make .hg/topic a directory, and release _that_?  Then
> >> >> > >> > everyone who uses zsh 5.9 with hg will be stuck with vcs_info errors
> >> >> > >> > until their distro upgrades to newer zsh.  
> >> >> > >>
> >> >> > >> The .hg/topic file works very similar to the .hg/branch file (which is
> >> >> > >> stable since 2007 and which zsh currently depends on). I can't think
> >> >> > >> of
> >> >> > >> any reason why someone would consider changing the format.
> >> >> > >>  
> >> >> > >
> >> >> > > I'm not going to second-guess your assessment, but I am surprised by
> >> >> > > it.  
> >> >> >
> >> >> > The main developer of the topic extension said that there will be clear
> >> >> > message if it changed. When I asked him "do you think it is safe for a
> >> >> > shell prompt hook to depend on the .hg/topic file name and data
> >> >> > format?", the answer was "safe enough". What he considers "safe enough"
> >> >> > might not be "safe enough" for zsh.  
> >> >>
> >> >> Thanks for going to the horse's mouth.  Yes, "safe enough" is a bit
> >> >> vague.
> >> >>  
> >> >> > As a zsh user, I find it reassuring that the standards for inclusion are high.  
> >> >>
> >> >> Thanks for your understanding and for making it explicit ☺
> >> >>  
> >> >> > >> The file would only be created if a user installs and activates the
> >> >> > >> extension, and explictly creates a topic (a specific kind of feature
> >> >> > >> branch), so that makes it even more unlikely that something breaks.  
> >> >> > >
> >> >> > > I do see that if topics are not popular, then the probability that
> >> >> > > a random hg user would be affected by a forward incompatible assumption
> >> >> > > in vcs_info is low.  However, users of hg topics that uses vcs_info
> >> >> > > _would_ be affected and would have no easy workaround.
> >> >> > >
> >> >> > > Furthermore, it is zsh, not hg, who would receive the consequent bug
> >> >> > > reports and negative word-of-mouth.
> >> >> > >
> >> >> > > Could you comment on what options are there to design forward
> >> >> > > compatibility into the patch?  It would be good to lay out
> >> >> > > alternatives,
> >> >> > > even if we don't end up implementing any of them.  
> >> >> >
> >> >> > One option is that people would have to opt-in to get the topic shown.
> >> >> > This way, people could be made aware that they’re using something that
> >> >> > could break in the future. However, I don’t know whether zsh provides a
> >> >> > framework for having this kind of configuration.
> >> >> >  
> >> >>
> >> >> Configurability could be provided via the 'zstyle' builtin.  Briefly,
> >> >> zstyles map strings ("contexts") to key-to-list-of-words maps by
> >> >> specifying pattern that contexts are matched against.  (See the manual
> >> >> for details.)  A user might do in their zshrc
> >> >> .
> >> >>     zstyle ':vcs_info:hg:*' experimental-features topic
> >> >> .
> >> >> and then the vcs_info backend would run «zstyle -a» and get an array
> >> >> «reply=( topic )».  Without that zshrc setting, the array would be
> >> >> empty.
> >> >>
> >> >> There should be plenty of examples of this in vcs_info, and more in
> >> >> the
> >> >> completion system.  
> >> 
> >> Do you think an explicit opt-in is necessary in combination with the
> >> other forward-compatibility options?  
> > 
> > Not necessarily.  For example, if hg upstream accepts a unit test for
> > the kind of parsing vcs_info does, I'd consider that reason enough to
> > dispense with the opt-in.
> > 
> > Alternatively, if there is a way to probe hg(1)'s output and/or the .hg
> > dir to see what version of the topic extension data format is used, and
> > if we can rely on that version number to be bumped if an incompatible
> > change are made, that too will remove the need for an opt-in.  
> 
> I don’t think that this specific file will be versioned. For most 
> imaginable versionable subsets of the topic extension that include this 
> file, I expect that the other parts cause much more frequent version 
> bumps than the format of this file is changed (as mentioned before, I 
> think the chance for this is very low and would be catched by the extra 
> checks in the next version of the patch).
> 

I see.  In this case, we'll just have to rely on the unit test in hg's
test suite.

> > The idea, in any case, is to be as forward compatible with future hg
> > as practicable.
> >   
> >> I’m concerned that most people won’t use the feature because they’re
> >> not aware that it exists.  
> > 
> > Fair point.  Let's first see if we can enable the feature without user
> > interaction at all.  If that doesn't pan out, then we can see about
> > addressing the lack of awareness.
> >   
> >> >> > The hook could check harder that the .hg/topic file is in the expected
> >> >> > format, e.g. checking that it is a regular file. As long as it’s a
> >> >> > regular file, I’d consider it a gross violation of the user expectation
> >> >> > if the file contained anything other than simply the topic name.
> >> >> >  
> >> >>
> >> >> Makes sense.  This way, if there _is_ an upstream
> >> >> backwards-incompatible
> >> >> change [which is fine, of course, in an experimental feature], the
> >> >> fallout would be minimal.
> >> >>
> >> >> Or maybe vcs_info could read just the first line of the file.  This
> >> >> way, if future hg adds more info in subsequent lines, we'll be forward
> >> >> compatible with that.  
> >> 
> >> Good idea. I’ll implement that.  
> > 
> > Looking forward to the revised patch, then.
> >   
> >> >> > I could submit the diff of this patch for inclusion in the "contrib"
> >> >> > directory of the topic extension repository to remind them that people
> >> >> > depend on the file name and data format.  
> >> >>
> >> >> Making upstream aware that it has downstreams that depend on the data
> >> >> format would definitely be a Good Thing.  A contrib patch wouldn't
> >> >> have
> >> >> been my first choice, though: I'd rather recommend creating a unit
> >> >> test
> >> >> that creates and reads a topic in the same way vcs_info will, with
> >> >> a comment explaining that (and a comment in vcs_info that points to
> >> >> the
> >> >> test).  
> >> 
> >> Indeed that sounds better than a patch in contrib. There’s currently 
> >> an
> >> unrelated large patch in-review for the repository the test would be
> >> added to. Not sure whether I can sneak it through on the side. I won’t
> >> block the zsh patch on that.  
> > 
> > +1
> >   
> >> >> Or perhaps the vcs_info hg backend should be maintained as part of hg?
> >> >> That would require vcs_info to provide a public API for third-party
> >> >> backends.  
> >> 
> >> I’m lacking the experience to change vcs_info to provide such public
> >> API.  
> > 
> > I wouldn't have expected you to make such a change as part of this 
> > patch
> > submission; I was merely brainstorming potential larger-scale changes
> > for the future.  To block the patch pending such changes would be to 
> > let
> > the perfect be the enemy of the good.
> >   
> >> The backend would be part of hg, while the code for writing the topic 
> >> to
> >> the file is in another repository. Therefore, the backend itself would
> >> need to provide additional hooks the topic extension could use.  
> > 
> > Sounds like we should keep vcs_info's design as-is, then.  Bugfixes 
> > that
> > require concerted changes across three repositories are an imperial 
> > pain.
> >   
> >> >> Cheers!
> >> >>
> >> >> Daniel  
> >> 
> >> Thank you for the superb review experience so far!  
> > 
> > You're welcome, and thanks for taking the forward compatibility 
> > concerns in stride!  
> 
> I hope I won’t forget anything obvious on the next iteration of the 
> patch.

And I hope I haven't missed anything obvious in reviewing it so far:
I've only eyeballed the patch, not tested it.  (Come to think of it,
I'm not sure how one would install the topic extension for testing the
patch.  I suppose I could just create .hg/topic by hand, but…)

> > Cheers,
> > 
> > Daniel
> > 
> > P.S.  Unrelated question: Where would you expect the existence of
> > a custom syntax highlighting file for the manual's yodl (*.yo) sources
> > to be documented?  We have such a file (Util/zyodl.vim), but I just
> > realized that I forgot to document its existence.  
> 
> Maybe you could mention it in the documentation for the format itself?

Thanks, but I don't see how.  The yodl format is an external project,
and the syntax file is specific to zsh's dialect of yodl: it highlights
some in-house macros (such as ifzman() and zmanref()) and doesn't
highlight standard yodl macros that zsh doesn't use.

Anyway, I see now that it's already documented in Etc/zsh-development-guide.
I'd completely forgotten about that, and my greps yesterday didn't find
it either.  (I used «ag zyodl», but ag(1)'s default excludes meant the
file with the match wasn't searched.)  Sorry.

Cheers,

Daniel