zsh-workers
 help / color / Atom feed
* [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
@ 2020-06-07  7:44 Manuel Jacob
  2020-06-07 12:14 ` Daniel Shahaf
  2020-06-07 13:49 ` Manuel Jacob
  0 siblings, 2 replies; 16+ messages in thread
From: Manuel Jacob @ 2020-06-07  7:44 UTC (permalink / raw)
  To: zsh-workers; +Cc: Manuel Jacob

"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..e898f7298 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 [[ -r ${topicfile} ]] ; then
+    r_branch=${r_branch}:$(< ${topicfile})
+fi
+
 # The working dir has uncommitted-changes if the revision ends with a +
 if [[ $r_lrev[-1] == + ]] ; then
     hgchanges=1
-- 
2.26.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-07  7:44 [PATCH] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
@ 2020-06-07 12:14 ` Daniel Shahaf
  2020-06-07 13:31   ` Manuel Jacob
  2020-06-07 13:49 ` Manuel Jacob
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-07 12:14 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

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

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.

Cheers,

Daniel

> ---
>  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..e898f7298 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 [[ -r ${topicfile} ]] ; then
> +    r_branch=${r_branch}:$(< ${topicfile})
> +fi
> +
>  # The working dir has uncommitted-changes if the revision ends with a +
>  if [[ $r_lrev[-1] == + ]] ; then
>      hgchanges=1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-07 12:14 ` Daniel Shahaf
@ 2020-06-07 13:31   ` Manuel Jacob
  2020-06-08  5:31     ` Daniel Shahaf
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel Jacob @ 2020-06-07 13:31 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

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.

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

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.

> Cheers,
> 
> Daniel
> 
>> ---
>>  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..e898f7298 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 [[ -r ${topicfile} ]] ; then
>> +    r_branch=${r_branch}:$(< ${topicfile})
>> +fi
>> +
>>  # The working dir has uncommitted-changes if the revision ends with a 
>> +
>>  if [[ $r_lrev[-1] == + ]] ; then
>>      hgchanges=1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-07  7:44 [PATCH] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
  2020-06-07 12:14 ` Daniel Shahaf
@ 2020-06-07 13:49 ` Manuel Jacob
  1 sibling, 0 replies; 16+ messages in thread
From: Manuel Jacob @ 2020-06-07 13:49 UTC (permalink / raw)
  To: zsh-workers

On 2020-06-07 09:44, Manuel Jacob wrote:
> "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..e898f7298 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 [[ -r ${topicfile} ]] ; then

I've sent a v2 of this patch that properly handles a zero .hg/topic 
file, which means "no topic set" (the same as no .hg/topic file).

> +    r_branch=${r_branch}:$(< ${topicfile})
> +fi
> +
>  # The working dir has uncommitted-changes if the revision ends with a 
> +
>  if [[ $r_lrev[-1] == + ]] ; then
>      hgchanges=1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-07 13:31   ` Manuel Jacob
@ 2020-06-08  5:31     ` Daniel Shahaf
  2020-06-09  1:15       ` Manuel Jacob
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-08  5:31 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

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

Cheers,

Daniel

> > Cheers,
> > 
> > Daniel
> >   
> >> ---
> >>  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..e898f7298 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 [[ -r ${topicfile} ]] ; then
> >> +    r_branch=${r_branch}:$(< ${topicfile})
> >> +fi
> >> +
> >>  # The working dir has uncommitted-changes if the revision ends with a 
> >> +
> >>  if [[ $r_lrev[-1] == + ]] ; then
> >>      hgchanges=1  


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-08  5:31     ` Daniel Shahaf
@ 2020-06-09  1:15       ` Manuel Jacob
  2020-06-09 22:34         ` Daniel Shahaf
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel Jacob @ 2020-06-09  1:15 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

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. As a zsh user, I find it reassuring 
that the standards for inclusion are high.

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

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.

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.

> Cheers,
> 
> Daniel
> 
>> > Cheers,
>> >
>> > Daniel
>> >
>> >> ---
>> >>  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..e898f7298 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 [[ -r ${topicfile} ]] ; then
>> >> +    r_branch=${r_branch}:$(< ${topicfile})
>> >> +fi
>> >> +
>> >>  # The working dir has uncommitted-changes if the revision ends with a
>> >> +
>> >>  if [[ $r_lrev[-1] == + ]] ; then
>> >>      hgchanges=1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-09  1:15       ` Manuel Jacob
@ 2020-06-09 22:34         ` Daniel Shahaf
  2020-06-17  8:27           ` Daniel Shahaf
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-09 22:34 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

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.

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

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

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.

Cheers!

Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-09 22:34         ` Daniel Shahaf
@ 2020-06-17  8:27           ` Daniel Shahaf
  2020-06-19  2:06             ` Manuel Jacob
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-17  8:27 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

Good morning Manuel,

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

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.
> 
> > 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.
> 
> > 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).
> 
> 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.
> 
> Cheers!
> 
> Daniel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-17  8:27           ` Daniel Shahaf
@ 2020-06-19  2:06             ` Manuel Jacob
  2020-06-19 10:07               ` Daniel Shahaf
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel Jacob @ 2020-06-19  2:06 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

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!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-19  2:06             ` Manuel Jacob
@ 2020-06-19 10:07               ` Daniel Shahaf
  2020-06-19 10:46                 ` vcs_info patch-format/applied-string awareness? (was: Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.) Daniel Shahaf
  2020-06-19 23:29                 ` [PATCH] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
  0 siblings, 2 replies; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-19 10:07 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

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.

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!

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* vcs_info patch-format/applied-string awareness? (was: Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.)
  2020-06-19 10:07               ` Daniel Shahaf
@ 2020-06-19 10:46                 ` Daniel Shahaf
  2020-06-19 22:48                   ` vcs_info patch-format/applied-string awareness? Manuel Jacob
  2020-06-19 23:29                 ` [PATCH] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-19 10:46 UTC (permalink / raw)
  To: Manuel Jacob, zsh-workers

Daniel Shahaf wrote on Fri, 19 Jun 2020 10:07 +0000:
> Manuel Jacob wrote on Fri, 19 Jun 2020 04:06 +0200:
> > 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.

By the way, I think this point applies to the "show patch series"
functionality too.  That functionality is used by several vcs backends
(e.g., quilt, hg mq, git rebase), but is not enabled by default and
I suspect many people don't know it exists.

I'm referring to this:

[[[
$ cd Functions/VCS_Info/
$ rm -rf vcs-test/ 
$ zsh ./test-repo-git-rebase-apply
⋮
CONFLICT (content): Merge conflict in iota
⋮
zsh: exit 1     zsh ./test-repo-git-rebase-apply
$ (cd vcs-test/ && zsh) 
myref/rebase/+ f973ee0a1fe5
[5+2=7] ea03c123a587 r7: Append a line
% 
]]]

See the "[5+2=7] ea03c123a587 r7: Append a line" line.  (That line was
generated by setting zstyles to values such as those given in comments
at the top of ./test-repo-git-rebase-apply, plus some fine-tuning by
zshrc hooks.)

So, should we have patch series information displayed by default?  The
information about applied patches is parsed by default, but not shown
by default because the %m expando isn't in the default formats/actionformats
values.

Or alternatively, should we increase awareness of it?

Cheers,

Daniel

P.S.  The hg backend doesn't seem to support equivalent functionality
for the rebase/histedit commands; see the VCS_INFO_set-patch-format call.
hg mq support is present but not enabled by default.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: vcs_info patch-format/applied-string awareness?
  2020-06-19 10:46                 ` vcs_info patch-format/applied-string awareness? (was: Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.) Daniel Shahaf
@ 2020-06-19 22:48                   ` Manuel Jacob
  2020-06-20  9:54                     ` Daniel Shahaf
  0 siblings, 1 reply; 16+ messages in thread
From: Manuel Jacob @ 2020-06-19 22:48 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 2020-06-19 12:46, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Fri, 19 Jun 2020 10:07 +0000:
>> Manuel Jacob wrote on Fri, 19 Jun 2020 04:06 +0200:
>> > 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.
> 
> By the way, I think this point applies to the "show patch series"
> functionality too.  That functionality is used by several vcs backends
> (e.g., quilt, hg mq, git rebase), but is not enabled by default and
> I suspect many people don't know it exists.
> 
> I'm referring to this:
> 
> [[[
> $ cd Functions/VCS_Info/
> $ rm -rf vcs-test/
> $ zsh ./test-repo-git-rebase-apply
> ⋮
> CONFLICT (content): Merge conflict in iota
> ⋮
> zsh: exit 1     zsh ./test-repo-git-rebase-apply
> $ (cd vcs-test/ && zsh)
> myref/rebase/+ f973ee0a1fe5
> [5+2=7] ea03c123a587 r7: Append a line
> %
> ]]]
> 
> See the "[5+2=7] ea03c123a587 r7: Append a line" line.  (That line was
> generated by setting zstyles to values such as those given in comments
> at the top of ./test-repo-git-rebase-apply, plus some fine-tuning by
> zshrc hooks.)
> 
> So, should we have patch series information displayed by default?  The
> information about applied patches is parsed by default, but not shown
> by default because the %m expando isn't in the default 
> formats/actionformats
> values.
> 
> Or alternatively, should we increase awareness of it?

MQ is deprecated and discouraged and any support for it doesn’t need any 
more awareness. Personally, I neither want to spend time thinking about 
how to improve MQ support nor answer any questions from MQ users that 
keep using it because zsh support improved. ;)

> Cheers,
> 
> Daniel
> 
> P.S.  The hg backend doesn't seem to support equivalent functionality
> for the rebase/histedit commands; see the VCS_INFO_set-patch-format 
> call.
> hg mq support is present but not enabled by default.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-19 10:07               ` Daniel Shahaf
  2020-06-19 10:46                 ` vcs_info patch-format/applied-string awareness? (was: Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.) Daniel Shahaf
@ 2020-06-19 23:29                 ` Manuel Jacob
  2020-06-20 10:43                   ` Daniel Shahaf
  1 sibling, 1 reply; 16+ messages in thread
From: Manuel Jacob @ 2020-06-19 23:29 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

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

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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: vcs_info patch-format/applied-string awareness?
  2020-06-19 22:48                   ` vcs_info patch-format/applied-string awareness? Manuel Jacob
@ 2020-06-20  9:54                     ` Daniel Shahaf
  2020-06-20 22:06                       ` Manuel Jacob
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-20  9:54 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

Manuel Jacob wrote on Fri, 19 Jun 2020 22:48 +00:00:
> On 2020-06-19 12:46, Daniel Shahaf wrote:
> > Daniel Shahaf wrote on Fri, 19 Jun 2020 10:07 +0000:
> >> Manuel Jacob wrote on Fri, 19 Jun 2020 04:06 +0200:
> >> > 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.
> > 
> > By the way, I think this point applies to the "show patch series"
> > functionality too.  That functionality is used by several vcs backends
> > (e.g., quilt, hg mq, git rebase), but is not enabled by default and
> > I suspect many people don't know it exists.
> > 
> > I'm referring to this:
> > 
> > [[[
> > $ cd Functions/VCS_Info/
> > $ rm -rf vcs-test/
> > $ zsh ./test-repo-git-rebase-apply
> > ⋮
> > CONFLICT (content): Merge conflict in iota
> > ⋮
> > zsh: exit 1     zsh ./test-repo-git-rebase-apply
> > $ (cd vcs-test/ && zsh)
> > myref/rebase/+ f973ee0a1fe5
> > [5+2=7] ea03c123a587 r7: Append a line
> > %
> > ]]]
> > 
> > See the "[5+2=7] ea03c123a587 r7: Append a line" line.  (That line was
> > generated by setting zstyles to values such as those given in comments
> > at the top of ./test-repo-git-rebase-apply, plus some fine-tuning by
> > zshrc hooks.)
> > 
> > So, should we have patch series information displayed by default?  The
> > information about applied patches is parsed by default, but not shown
> > by default because the %m expando isn't in the default 
> > formats/actionformats
> > values.
> > 
> > Or alternatively, should we increase awareness of it?
> 
> MQ is deprecated and discouraged and any support for it doesn’t need any 
> more awareness. Personally, I neither want to spend time thinking about 
> how to improve MQ support nor answer any questions from MQ users that 
> keep using it because zsh support improved. ;)

Fair enough as far as the hg backend is concerned.

However, my question was not specific to the hg backend; I was expressly
asking also about the git and quilt backends, and my question stands
with respect to them.

Cheers,

Daniel

> > Cheers,
> > 
> > Daniel
> > 
> > P.S.  The hg backend doesn't seem to support equivalent
> > functionality for the rebase/histedit commands; see the VCS_INFO_set-patch-format
> > call. hg mq support is present but not enabled by default.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.
  2020-06-19 23:29                 ` [PATCH] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
@ 2020-06-20 10:43                   ` Daniel Shahaf
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Shahaf @ 2020-06-20 10:43 UTC (permalink / raw)
  To: Manuel Jacob; +Cc: zsh-workers

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: vcs_info patch-format/applied-string awareness?
  2020-06-20  9:54                     ` Daniel Shahaf
@ 2020-06-20 22:06                       ` Manuel Jacob
  0 siblings, 0 replies; 16+ messages in thread
From: Manuel Jacob @ 2020-06-20 22:06 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 2020-06-20 11:54, Daniel Shahaf wrote:
> Manuel Jacob wrote on Fri, 19 Jun 2020 22:48 +00:00:
>> On 2020-06-19 12:46, Daniel Shahaf wrote:
>> > Daniel Shahaf wrote on Fri, 19 Jun 2020 10:07 +0000:
>> >> Manuel Jacob wrote on Fri, 19 Jun 2020 04:06 +0200:
>> >> > 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.
>> >
>> > By the way, I think this point applies to the "show patch series"
>> > functionality too.  That functionality is used by several vcs backends
>> > (e.g., quilt, hg mq, git rebase), but is not enabled by default and
>> > I suspect many people don't know it exists.
>> >
>> > I'm referring to this:
>> >
>> > [[[
>> > $ cd Functions/VCS_Info/
>> > $ rm -rf vcs-test/
>> > $ zsh ./test-repo-git-rebase-apply
>> > ⋮
>> > CONFLICT (content): Merge conflict in iota
>> > ⋮
>> > zsh: exit 1     zsh ./test-repo-git-rebase-apply
>> > $ (cd vcs-test/ && zsh)
>> > myref/rebase/+ f973ee0a1fe5
>> > [5+2=7] ea03c123a587 r7: Append a line
>> > %
>> > ]]]
>> >
>> > See the "[5+2=7] ea03c123a587 r7: Append a line" line.  (That line was
>> > generated by setting zstyles to values such as those given in comments
>> > at the top of ./test-repo-git-rebase-apply, plus some fine-tuning by
>> > zshrc hooks.)
>> >
>> > So, should we have patch series information displayed by default?  The
>> > information about applied patches is parsed by default, but not shown
>> > by default because the %m expando isn't in the default
>> > formats/actionformats
>> > values.
>> >
>> > Or alternatively, should we increase awareness of it?
>> 
>> MQ is deprecated and discouraged and any support for it doesn’t need 
>> any
>> more awareness. Personally, I neither want to spend time thinking 
>> about
>> how to improve MQ support nor answer any questions from MQ users that
>> keep using it because zsh support improved. ;)
> 
> Fair enough as far as the hg backend is concerned.
> 
> However, my question was not specific to the hg backend; I was 
> expressly
> asking also about the git and quilt backends, and my question stands
> with respect to them.

I neither use MQ nor quilt, as modern Mercurial makes history editing 
very nice.

For rebase, I see how it could be useful, but not as much as showing the 
current revision or branch. If I hit a merge conflict, I will usually 
try to solve it as soon as possible, without context-switching. After a 
context-switch and reopening a terminal in some directory, the first 
thing I want to know is where I am.

When reordering a stack of changesets, I use histedit only if I have a 
clear idea beforehand of how I want it to look like or if I want to 
mass-edit some commit messages.

When doing something more complex, I have a GUI (TortoiseHg) open and do 
it stepwise. With modern Mercurial (with changeset evolution), you can 
rebase single changesets. The GUI will show which changeset "obsoleted" 
which changeset. The descendants of the obsoleted changesets will be 
shown as "orphan". Piece-by-piece the new stack grows while old stack 
with the orphans gets smaller. During that, I usually have a clear idea 
of where I am. When solving merge conflicts or editing some revision, 
I’ll usually do it quickly without context-switches, so I don’t really 
have to be reminded.

When doing it stepwise, showing the current position in the "patch 
series" is not as useful as it is with `git rebase -i` or `hg histedit`. 
For showing the whole branch, there’s the GUI or the `hg stack` command.

> Cheers,
> 
> Daniel
> 
>> > Cheers,
>> >
>> > Daniel
>> >
>> > P.S.  The hg backend doesn't seem to support equivalent
>> > functionality for the rebase/histedit commands; see the VCS_INFO_set-patch-format
>> > call. hg mq support is present but not enabled by default.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, back to index

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-07  7:44 [PATCH] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
2020-06-07 12:14 ` Daniel Shahaf
2020-06-07 13:31   ` Manuel Jacob
2020-06-08  5:31     ` Daniel Shahaf
2020-06-09  1:15       ` Manuel Jacob
2020-06-09 22:34         ` Daniel Shahaf
2020-06-17  8:27           ` Daniel Shahaf
2020-06-19  2:06             ` Manuel Jacob
2020-06-19 10:07               ` Daniel Shahaf
2020-06-19 10:46                 ` vcs_info patch-format/applied-string awareness? (was: Re: [PATCH] Add code to Mercurial VCS backend to show topic if there is any.) Daniel Shahaf
2020-06-19 22:48                   ` vcs_info patch-format/applied-string awareness? Manuel Jacob
2020-06-20  9:54                     ` Daniel Shahaf
2020-06-20 22:06                       ` Manuel Jacob
2020-06-19 23:29                 ` [PATCH] Add code to Mercurial VCS backend to show topic if there is any Manuel Jacob
2020-06-20 10:43                   ` Daniel Shahaf
2020-06-07 13:49 ` Manuel Jacob

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git