Patchwork [2,of,3] revset: implement copies and renames for checkstatus

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date April 5, 2019, 6:42 p.m.
Message ID <4baa10f1f44a8e427f49.1554489758@chloe>
Download mbox | patch
Permalink /patch/39515/
State Accepted
Headers show

Comments

Jordi Gutiérrez Hermoso - April 5, 2019, 6:42 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1554489104 14400
#      Fri Apr 05 14:31:44 2019 -0400
# Node ID 4baa10f1f44a8e427f49fa4f4d8d29552c2a1a65
# Parent  9fcb915a73b83547921aaa13584c88cb99c6aee7
revset: implement copies and renames for checkstatus

Determining when a file is a copy is tricky and isn't handled by the
normal status functions, so thankfully we can offload that work to
the copies module, just like the status command itself does.
via Mercurial-devel - April 6, 2019, 1:34 a.m.
On Fri, Apr 5, 2019, 12:30 Jordi Gutiérrez Hermoso <jordigh@octave.org>
wrote:

> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1554489104 14400
> #      Fri Apr 05 14:31:44 2019 -0400
> # Node ID 4baa10f1f44a8e427f49fa4f4d8d29552c2a1a65
> # Parent  9fcb915a73b83547921aaa13584c88cb99c6aee7
> revset: implement copies and renames for checkstatus
>
> Determining when a file is a copy is tricky and isn't handled by the
> normal status functions, so thankfully we can offload that work to
> the copies module, just like the status command itself does.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -11,6 +11,7 @@ import re
>
>  from .i18n import _
>  from . import (
> +    copies as copiesmod,
>      dagop,
>      destutil,
>      diffutil,
> @@ -603,6 +604,8 @@ def checkstatus(repo, subset, pat, field
>      0: modified
>      1: added
>      2: removed
> +    3: copied (not renamed, i.e. source not removed)
> +    4: renamed (not copied, i..e source removed)
>

Misplaced '.' (and/or 'e' :) ) in "i.e."

     """
>      hasset = matchmod.patkind(pat) == 'set'
>
> @@ -624,7 +627,18 @@ def checkstatus(repo, subset, pat, field
>                      break
>              else:
>                  return False
> -        files = repo.status(c.p1().node(), c.node())[field]
> +        p1 = c.p1()
> +        status = repo.status(p1.node(), c.node())
> +        if field == 3:
> +            copymap = copiesmod.pathcopies(p1, c, m)
> +            removed = status[2]
>

"status.removed" is easier to read

+            files = [dest for (dest, src) in copymap.items() if src not in
> removed]
> +        elif field == 4:
> +            copymap = copiesmod.pathcopies(p1, c, m)
> +            removed = status[2]
> +            files = [dest for (dest, src) in copymap.items() if src in
> removed]
> +        else:
> +            files = status[field]
>          if fname is not None:
>              if fname in files:
>                  return True
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - April 7, 2019, 12:55 a.m.
On Fri, 05 Apr 2019 14:42:38 -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1554489104 14400
> #      Fri Apr 05 14:31:44 2019 -0400
> # Node ID 4baa10f1f44a8e427f49fa4f4d8d29552c2a1a65
> # Parent  9fcb915a73b83547921aaa13584c88cb99c6aee7
> revset: implement copies and renames for checkstatus
> 
> Determining when a file is a copy is tricky and isn't handled by the
> normal status functions, so thankfully we can offload that work to
> the copies module, just like the status command itself does.
> 
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -11,6 +11,7 @@ import re
>  
>  from .i18n import _
>  from . import (
> +    copies as copiesmod,
>      dagop,
>      destutil,
>      diffutil,
> @@ -603,6 +604,8 @@ def checkstatus(repo, subset, pat, field
>      0: modified
>      1: added
>      2: removed
> +    3: copied (not renamed, i.e. source not removed)
> +    4: renamed (not copied, i..e source removed)
>      """
>      hasset = matchmod.patkind(pat) == 'set'
>  
> @@ -624,7 +627,18 @@ def checkstatus(repo, subset, pat, field
>                      break
>              else:
>                  return False
> -        files = repo.status(c.p1().node(), c.node())[field]
> +        p1 = c.p1()
> +        status = repo.status(p1.node(), c.node())
> +        if field == 3:
> +            copymap = copiesmod.pathcopies(p1, c, m)
> +            removed = status[2]
> +            files = [dest for (dest, src) in copymap.items() if src not in removed]
> +        elif field == 4:
> +            copymap = copiesmod.pathcopies(p1, c, m)
> +            removed = status[2]
> +            files = [dest for (dest, src) in copymap.items() if src in removed]
> +        else:
> +            files = status[field]

Maybe we can turn the field argument into a lambda function. It doesn't make
sense to introduce pseudo indices for copies and renames.
Jordi Gutiérrez Hermoso - April 8, 2019, 6:27 p.m.
On Sun, 2019-04-07 at 09:55 +0900, Yuya Nishihara wrote:
> On Fri, 05 Apr 2019 14:42:38 -0400, Jordi Gutiérrez Hermoso wrote:

> > @@ -624,7 +627,18 @@ def checkstatus(repo, subset, pat, field
> >                      break
> >              else:
> >                  return False
> > -        files = repo.status(c.p1().node(), c.node())[field]
> > +        p1 = c.p1()
> > +        status = repo.status(p1.node(), c.node())
> > +        if field == 3:
> > +            copymap = copiesmod.pathcopies(p1, c, m)
> > +            removed = status[2]
> > +            files = [dest for (dest, src) in copymap.items() if src not in removed]
> > +        elif field == 4:
> > +            copymap = copiesmod.pathcopies(p1, c, m)
> > +            removed = status[2]
> > +            files = [dest for (dest, src) in copymap.items() if src in removed]
> > +        else:
> > +            files = status[field]
> 
> Maybe we can turn the field argument into a lambda function. It doesn't make
> sense to introduce pseudo indices for copies and renames.

I don't understand what you mean. Are you saing that the caller should
pass a function that should say how to get the appropriate data out of
the status object or copymap objects?
Yuya Nishihara - April 8, 2019, 11:27 p.m.
On Mon, 08 Apr 2019 14:27:53 -0400, Jordi Gutiérrez Hermoso wrote:
> On Sun, 2019-04-07 at 09:55 +0900, Yuya Nishihara wrote:
> > On Fri, 05 Apr 2019 14:42:38 -0400, Jordi Gutiérrez Hermoso wrote:
> 
> > > @@ -624,7 +627,18 @@ def checkstatus(repo, subset, pat, field
> > >                      break
> > >              else:
> > >                  return False
> > > -        files = repo.status(c.p1().node(), c.node())[field]
> > > +        p1 = c.p1()
> > > +        status = repo.status(p1.node(), c.node())
> > > +        if field == 3:
> > > +            copymap = copiesmod.pathcopies(p1, c, m)
> > > +            removed = status[2]
> > > +            files = [dest for (dest, src) in copymap.items() if src not in removed]
> > > +        elif field == 4:
> > > +            copymap = copiesmod.pathcopies(p1, c, m)
> > > +            removed = status[2]
> > > +            files = [dest for (dest, src) in copymap.items() if src in removed]
> > > +        else:
> > > +            files = status[field]
> > 
> > Maybe we can turn the field argument into a lambda function. It doesn't make
> > sense to introduce pseudo indices for copies and renames.
> 
> I don't understand what you mean. Are you saing that the caller should
> pass a function that should say how to get the appropriate data out of
> the status object or copymap objects?

Something like that. My point is that checkstatus(..., getcopiedfiles) will
be more readable than checkstatus(..., 3), which sounds like returning the
3rd status field.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -11,6 +11,7 @@  import re
 
 from .i18n import _
 from . import (
+    copies as copiesmod,
     dagop,
     destutil,
     diffutil,
@@ -603,6 +604,8 @@  def checkstatus(repo, subset, pat, field
     0: modified
     1: added
     2: removed
+    3: copied (not renamed, i.e. source not removed)
+    4: renamed (not copied, i..e source removed)
     """
     hasset = matchmod.patkind(pat) == 'set'
 
@@ -624,7 +627,18 @@  def checkstatus(repo, subset, pat, field
                     break
             else:
                 return False
-        files = repo.status(c.p1().node(), c.node())[field]
+        p1 = c.p1()
+        status = repo.status(p1.node(), c.node())
+        if field == 3:
+            copymap = copiesmod.pathcopies(p1, c, m)
+            removed = status[2]
+            files = [dest for (dest, src) in copymap.items() if src not in removed]
+        elif field == 4:
+            copymap = copiesmod.pathcopies(p1, c, m)
+            removed = status[2]
+            files = [dest for (dest, src) in copymap.items() if src in removed]
+        else:
+            files = status[field]
         if fname is not None:
             if fname in files:
                 return True