Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

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

On 2020-06-17 10:27, Daniel Shahaf wrote:

Good morning Manuel,

Is this still somewhere on your todo list?  Any questions to us?


Hi Daniel,
I started a few too many things at once. Sorry for letting you wait and for keeping it in your head for a longer than necessary. I answer the questions inline below. 


No rush.

Cheers,

Daniel


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? I’m concerned that most people won’t use the feature because they’re not aware that it exists. 


> 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.


> 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. 


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.
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. 


Cheers!

Daniel


Thank you for the superb review experience so far!
Messages sorted by: Reverse Date, Date, Thread, Author