Patchwork keyword: handle filectx _customcmp

login
register
mail settings
Submitter Christian Ebert
Date Oct. 17, 2016, 3:52 p.m.
Message ID <ebef6b5593fa812e907f.1476719563@1.0.0.127.in-addr.arpa>
Download mbox | patch
Permalink /patch/17154/
State Deferred
Headers show

Comments

Christian Ebert - Oct. 17, 2016, 3:52 p.m.
# HG changeset patch
# User Christian Ebert <blacktrash@gmx.net>
# Date 1476718966 -7200
#      Mon Oct 17 17:42:46 2016 +0200
# Node ID ebef6b5593fa812e907fb4dae920a0c8b2ee00a0
# Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
keyword: handle filectx _customcmp

Related to issue5364.
Pierre-Yves David - Oct. 17, 2016, 10:27 p.m.
On 10/17/2016 05:52 PM, Christian Ebert wrote:
> # HG changeset patch
> # User Christian Ebert <blacktrash@gmx.net>
> # Date 1476718966 -7200
> #      Mon Oct 17 17:42:46 2016 +0200
> # Node ID ebef6b5593fa812e907fb4dae920a0c8b2ee00a0
> # Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
> keyword: handle filectx _customcmp

I'm not sure why this is an improvement. Can you elaborate on what this 
changes do and why this is the way to go ?

Adding a test case to show what it fixes would be helpful.

> Related to issue5364.
>
> diff --git a/hgext/keyword.py b/hgext/keyword.py
> --- a/hgext/keyword.py
> +++ b/hgext/keyword.py
> @@ -737,6 +737,8 @@ def reposetup(ui, repo):
>              return ret
>
>      def kwfilectx_cmp(orig, self, fctx):
> +        if fctx._customcmp:
> +            return fctx.cmp(self)
>          # keyword affects data size, comparing wdir and filelog size does
>          # not make sense
>          if (fctx._filenode is None and

Cheers,
Christian Ebert - Oct. 20, 2016, 9:58 p.m.
* Pierre-Yves David on Tuesday, October 18, 2016 at 00:27:21 +0200
> On 10/17/2016 05:52 PM, Christian Ebert wrote:
>> # HG changeset patch
>> # User Christian Ebert <blacktrash@gmx.net>
>> # Date 1476718966 -7200
>> #      Mon Oct 17 17:42:46 2016 +0200
>> # Node ID ebef6b5593fa812e907fb4dae920a0c8b2ee00a0
>> # Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
>> keyword: handle filectx _customcmp
> 
> I'm not sure why this is an improvement. Can you elaborate on what
> this changes do and why this is the way to go ?

It was suggested by Yuya:
https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089461.html

> Adding a test case to show what it fixes would be helpful.
> 
>> Related to issue5364.

I have no scenario where this has come up. The scenario of
issue5364 was caused by an outdated version of the extension. I
presume it could be seen as a precaution as kwfilectx_cmp
basically mimics filectx.cmp.

I already admitted cluelessnees regarding the _customcmp idea, so
for lack of time to dig deeper I relied on Yuya's judgement.
Pierre-Yves David - Oct. 28, 2016, 8:35 a.m.
On 10/20/2016 11:58 PM, Christian Ebert wrote:
> * Pierre-Yves David on Tuesday, October 18, 2016 at 00:27:21 +0200
>> On 10/17/2016 05:52 PM, Christian Ebert wrote:
>>> # HG changeset patch
>>> # User Christian Ebert <blacktrash@gmx.net>
>>> # Date 1476718966 -7200
>>> #      Mon Oct 17 17:42:46 2016 +0200
>>> # Node ID ebef6b5593fa812e907fb4dae920a0c8b2ee00a0
>>> # Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
>>> keyword: handle filectx _customcmp
>>
>> I'm not sure why this is an improvement. Can you elaborate on what
>> this changes do and why this is the way to go ?
>
> It was suggested by Yuya:
> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089461.html

Ha okay, I suggest mentioning that in the description next time.

>> Adding a test case to show what it fixes would be helpful.
>>
>>> Related to issue5364.
>
> I have no scenario where this has come up. The scenario of
> issue5364 was caused by an outdated version of the extension. I
> presume it could be seen as a precaution as kwfilectx_cmp
> basically mimics filectx.cmp.
>
> I already admitted cluelessnees regarding the _customcmp idea, so
> for lack of time to dig deeper I relied on Yuya's judgement.

Relying on yuya judgement is usually a good call ^^

However, the 4.0 freeze is in place and that does not seems suitable for 
stable, can you resend this when 4.0 is release on November 4.0 ?

Cheers,
Christian Ebert - Nov. 1, 2016, 5:39 a.m.
* Pierre-Yves David on Friday, October 28, 2016 at 10:35:01 +0200
> On 10/20/2016 11:58 PM, Christian Ebert wrote:
>> * Pierre-Yves David on Tuesday, October 18, 2016 at 00:27:21 +0200
>>> On 10/17/2016 05:52 PM, Christian Ebert wrote:
>>>> # HG changeset patch
>>>> # User Christian Ebert <blacktrash@gmx.net>
>>>> # Date 1476718966 -7200
>>>> #      Mon Oct 17 17:42:46 2016 +0200
>>>> # Node ID ebef6b5593fa812e907fb4dae920a0c8b2ee00a0
>>>> # Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
>>>> keyword: handle filectx _customcmp
>>> 
>>> I'm not sure why this is an improvement. Can you elaborate on what
>>> this changes do and why this is the way to go ?
>> 
>> It was suggested by Yuya:
>> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089461.html
> 
> Ha okay, I suggest mentioning that in the description next time.

Will do.

>>> Adding a test case to show what it fixes would be helpful.
>>> 
>>>> Related to issue5364.
>> 
>> I have no scenario where this has come up. The scenario of
>> issue5364 was caused by an outdated version of the extension. I
>> presume it could be seen as a precaution as kwfilectx_cmp
>> basically mimics filectx.cmp.
>> 
>> I already admitted cluelessnees regarding the _customcmp idea, so
>> for lack of time to dig deeper I relied on Yuya's judgement.
> 
> Relying on yuya judgement is usually a good call ^^
> 
> However, the 4.0 freeze is in place and that does not seems suitable
> for stable, can you resend this when 4.0 is release on November 4.0 ?

Sure.

Patch

diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -737,6 +737,8 @@  def reposetup(ui, repo):
             return ret
 
     def kwfilectx_cmp(orig, self, fctx):
+        if fctx._customcmp:
+            return fctx.cmp(self)
         # keyword affects data size, comparing wdir and filelog size does
         # not make sense
         if (fctx._filenode is None and