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 10114 invoked from network); 20 Jun 2020 10:44:21 -0000 Received: from ns1.primenet.com.au (HELO primenet.com.au) (203.24.36.2) by inbox.vuxu.org with ESMTPUTF8; 20 Jun 2020 10:44:21 -0000 Received: (qmail 9025 invoked by alias); 20 Jun 2020 10:43:59 -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: 46085 Received: (qmail 14753 invoked by uid 1010); 20 Jun 2020 10:43:59 -0000 X-Qmail-Scanner-Diagnostics: from out4-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.3/25842. spamassassin: 3.4.4. Clear:RC:0(66.111.4.28):SA:0(-2.6/5.0):. Processed in 3.770118 secs); 20 Jun 2020 10:43:59 -0000 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrudejkedgfeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkjghfofggtgfgsehtqh dttdertdejnecuhfhrohhmpeffrghnihgvlhcuufhhrghhrghfuceougdrshesuggrnhhi vghlrdhshhgrhhgrfhdrnhgrmhgvqeenucggtffrrghtthgvrhhnpeektdefleegvefhue efteeifeevudekgfehgeevfeduffetledvtedvffdvueekudenucffohhmrghinhepmhgv rhgtuhhrihgrlhdqshgtmhdrohhrghenucfkphepjeelrddujeeirdefledrieelnecuve hluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepugdrshesuggr nhhivghlrdhshhgrhhgrfhdrnhgrmhgv X-ME-Proxy: Date: Sat, 20 Jun 2020 10:43:16 +0000 From: Daniel Shahaf To: Manuel Jacob Cc: zsh-workers@zsh.org Subject: Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any. Message-ID: <20200620104316.69eccdc8@tarpaulin.shahaf.local2> In-Reply-To: <758143b10f8d8b495593fc5421e88c3d@manueljacob.de> References: <20200607074445.7723-1-me@manueljacob.de> <20200607121443.7c58ee16@tarpaulin.shahaf.local2> <20200608053140.33e79354@tarpaulin.shahaf.local2> <538786d4a3afe4b9432a1469ebafcad8@manueljacob.de> <20200609223416.59a43074@tarpaulin.shahaf.local2> <20200617082743.12b5d092@tarpaulin.shahaf.local2> <20200619100731.099deac3@tarpaulin.shahaf.local2> <758143b10f8d8b495593fc5421e88c3d@manueljacob.de> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: =20 > >> On 2020-06-17 10:27, Daniel Shahaf wrote: =20 > >> > Daniel Shahaf wrote on Tue, 09 Jun 2020 22:34 +0000: =20 > >> >> Manuel Jacob wrote on Tue, 09 Jun 2020 03:15 +0200: =20 > >> >> > On 2020-06-08 07:31, Daniel Shahaf wrote: =20 > >> >> > > Manuel Jacob wrote on Sun, 07 Jun 2020 15:31 +0200: =20 > >> >> > >> Hi Daniel, > >> >> > >> > >> >> > >> On 2020-06-07 14:14, Daniel Shahaf wrote: =20 > >> >> > >> > Manuel Jacob wrote on Sun, 07 Jun 2020 09:44 +0200: =20 > >> >> > >> >> "Topics" is an experimental concept in Mercurial that augme= nts the > >> >> > >> >> current branching concept (called "named branches"). > >> >> > >> >> > >> >> > >> >> For more information, see the not always up-to-date Mercuri= al Wiki > >> >> > >> >> page > >> >> > >> >> https://www.mercurial-scm.org/wiki/TopicPlan. =20 > >> >> > >> > > >> >> > >> > I assume "experimental" means "future releases of Mercurial = are not > >> >> > >> > promised to be backwards compatible with the current design"= . =20 > >> >> > >> > >> >> > >> It is not yet part of Mercurial itself, but developed separate= ly as an > >> >> > >> extension under the Mercurial project roof. > >> >> > >> =20 > >> >> > > > >> >> > > Thanks for the info. In practice, though, it doesn't make that= much > >> >> > > difference to vcs_info whether the functionality lives in Mercu= rial > >> >> > > core > >> >> > > or in an extension developed by the same development team. I w= as more > >> >> > > concerned with whether the current on-disk data format is promi= sed to > >> >> > > be > >> >> > > supported by future releases of the functionality [whether thos= e be in > >> >> > > hg core or in extensions]. > >> >> > > =20 > >> >> > >> > Is the .hg/topic file name and data format set in stone yet? > >> >> > >> > > >> >> > >> > What if zsh 5.9 is released and then the Mercurial developer= s 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_inf= o errors > >> >> > >> > until their distro upgrades to newer zsh. =20 > >> >> > >> > >> >> > >> 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. > >> >> > >> =20 > >> >> > > > >> >> > > I'm not going to second-guess your assessment, but I am surpris= ed by > >> >> > > it. =20 > >> >> > > >> >> > 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 e= nough" > >> >> > might not be "safe enough" for zsh. =20 > >> >> > >> >> Thanks for going to the horse's mouth. Yes, "safe enough" is a bit > >> >> vague. > >> >> =20 > >> >> > As a zsh user, I find it reassuring that the standards for inclus= ion are high. =20 > >> >> > >> >> Thanks for your understanding and for making it explicit =E2=98=BA > >> >> =20 > >> >> > >> The file would only be created if a user installs and activate= s the > >> >> > >> extension, and explictly creates a topic (a specific kind of f= eature > >> >> > >> branch), so that makes it even more unlikely that something br= eaks. =20 > >> >> > > > >> >> > > I do see that if topics are not popular, then the probability t= hat > >> >> > > a random hg user would be affected by a forward incompatible as= sumption > >> >> > > 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 consequen= t 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. =20 > >> >> > > >> >> > One option is that people would have to opt-in to get the topic s= hown. > >> >> > This way, people could be made aware that they=E2=80=99re using s= omething that > >> >> > could break in the future. However, I don=E2=80=99t know whether = zsh provides a > >> >> > framework for having this kind of configuration. > >> >> > =20 > >> >> > >> >> Configurability could be provided via the 'zstyle' builtin. Briefl= y, > >> >> zstyles map strings ("contexts") to key-to-list-of-words maps by > >> >> specifying pattern that contexts are matched against. (See the man= ual > >> >> for details.) A user might do in their zshrc > >> >> . > >> >> zstyle ':vcs_info:hg:*' experimental-features topic > >> >> . > >> >> and then the vcs_info backend would run =C2=ABzstyle -a=C2=BB and g= et an array > >> >> =C2=ABreply=3D( topic )=C2=BB. Without that zshrc setting, the arr= ay would be > >> >> empty. > >> >> > >> >> There should be plenty of examples of this in vcs_info, and more in > >> >> the > >> >> completion system. =20 > >>=20 > >> Do you think an explicit opt-in is necessary in combination with the > >> other forward-compatibility options? =20 > >=20 > > 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. > >=20 > > 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. =20 >=20 > I don=E2=80=99t think that this specific file will be versioned. For most= =20 > imaginable versionable subsets of the topic extension that include this=20 > file, I expect that the other parts cause much more frequent version=20 > bumps than the format of this file is changed (as mentioned before, I=20 > think the chance for this is very low and would be catched by the extra=20 > checks in the next version of the patch). >=20 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. > > =20 > >> I=E2=80=99m concerned that most people won=E2=80=99t use the feature b= ecause they=E2=80=99re > >> not aware that it exists. =20 > >=20 > > 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. > > =20 > >> >> > The hook could check harder that the .hg/topic file is in the exp= ected > >> >> > format, e.g. checking that it is a regular file. As long as it=E2= =80=99s a > >> >> > regular file, I=E2=80=99d consider it a gross violation of the us= er expectation > >> >> > if the file contained anything other than simply the topic name. > >> >> > =20 > >> >> > >> >> 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 forw= ard > >> >> compatible with that. =20 > >>=20 > >> Good idea. I=E2=80=99ll implement that. =20 > >=20 > > Looking forward to the revised patch, then. > > =20 > >> >> > I could submit the diff of this patch for inclusion in the "contr= ib" > >> >> > directory of the topic extension repository to remind them that p= eople > >> >> > depend on the file name and data format. =20 > >> >> > >> >> Making upstream aware that it has downstreams that depend on the da= ta > >> >> 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). =20 > >>=20 > >> Indeed that sounds better than a patch in contrib. There=E2=80=99s cur= rently=20 > >> 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= =E2=80=99t > >> block the zsh patch on that. =20 > >=20 > > +1 > > =20 > >> >> 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. =20 > >>=20 > >> I=E2=80=99m lacking the experience to change vcs_info to provide such = public > >> API. =20 > >=20 > > I wouldn't have expected you to make such a change as part of this=20 > > patch > > submission; I was merely brainstorming potential larger-scale changes > > for the future. To block the patch pending such changes would be to=20 > > let > > the perfect be the enemy of the good. > > =20 > >> The backend would be part of hg, while the code for writing the topic= =20 > >> to > >> the file is in another repository. Therefore, the backend itself would > >> need to provide additional hooks the topic extension could use. =20 > >=20 > > Sounds like we should keep vcs_info's design as-is, then. Bugfixes=20 > > that > > require concerted changes across three repositories are an imperial=20 > > pain. > > =20 > >> >> Cheers! > >> >> > >> >> Daniel =20 > >>=20 > >> Thank you for the superb review experience so far! =20 > >=20 > > You're welcome, and thanks for taking the forward compatibility=20 > > concerns in stride! =20 >=20 > I hope I won=E2=80=99t forget anything obvious on the next iteration of t= he=20 > 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=E2=80=A6) > > Cheers, > >=20 > > Daniel > >=20 > > 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. =20 >=20 > 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 =C2=ABag zyodl=C2=BB, but ag(1)'s default excludes mean= t the file with the match wasn't searched.) Sorry. Cheers, Daniel