D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG2f5a135b2b04: annotate: track whether a particular 
annotation was the result of a skip (authored by sid0, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D899?vs=2331=2341

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

AFFECTED FILES
  mercurial/context.py
  tests/test-annotate.py

CHANGE DETAILS

diff --git a/tests/test-annotate.py b/tests/test-annotate.py
--- a/tests/test-annotate.py
+++ b/tests/test-annotate.py
@@ -80,20 +80,22 @@
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
+annotateline('old', 2, True),
+# note that this line was carried over from earlier so it is *not*
+# marked skipped
 annotateline('p2', 2),
-annotateline('p2', 2),
+annotateline('p2', 2, True),
 annotateline('p2', 3),
 ])
 
 childann = decorate(childdata, childfctx)
 childann = _annotatepair([p2ann, p1ann], childfctx, childann, True,
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
+annotateline('old', 2, True),
 annotateline('p1', 3),
-annotateline('p1', 3),
+annotateline('p1', 3, True),
 annotateline('p2', 3),
 ])
 
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -,6 +,8 @@
 class annotateline(object):
 fctx = attr.ib()
 lineno = attr.ib(default=False)
+# Whether this annotation was the result of a skip-annotate.
+skip = attr.ib(default=False)
 
 def _annotatepair(parents, childfctx, child, skipchild, diffopts):
 r'''
@@ -1159,7 +1161,7 @@
 for bk in xrange(b1, b2):
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
-child[0][bk] = parent[0][ak]
+child[0][bk] = attr.evolve(parent[0][ak], 
skip=True)
 else:
 remaining[idx][1].append((a1, a2, b1, b2))
 
@@ -1170,7 +1172,7 @@
 for bk in xrange(b1, b2):
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
-child[0][bk] = parent[0][ak]
+child[0][bk] = attr.evolve(parent[0][ak], skip=True)
 return child
 
 class filectx(basefilectx):



To: sid0, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread indygreg (Gregory Szorc) 
indygreg accepted this revision.
indygreg added a comment.
This revision is now accepted and ready to land.


  Immutability is a magical property, isn't it ;)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

To: sid0, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
sid0 added a comment.


  Turns out it actually was buggy. Thanks for catching it!
  
  I switched to using `attrs.evolve` (and in the earlier diff setting 
`frozen=True` to make the attr immutable)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

To: sid0, #hg-reviewers
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
sid0 updated this revision to Diff 2331.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D899?vs=2329=2331

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

AFFECTED FILES
  mercurial/context.py
  tests/test-annotate.py

CHANGE DETAILS

diff --git a/tests/test-annotate.py b/tests/test-annotate.py
--- a/tests/test-annotate.py
+++ b/tests/test-annotate.py
@@ -80,20 +80,22 @@
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
+annotateline('old', 2, True),
+# note that this line was carried over from earlier so it is *not*
+# marked skipped
 annotateline('p2', 2),
-annotateline('p2', 2),
+annotateline('p2', 2, True),
 annotateline('p2', 3),
 ])
 
 childann = decorate(childdata, childfctx)
 childann = _annotatepair([p2ann, p1ann], childfctx, childann, True,
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
+annotateline('old', 2, True),
 annotateline('p1', 3),
-annotateline('p1', 3),
+annotateline('p1', 3, True),
 annotateline('p2', 3),
 ])
 
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -,6 +,8 @@
 class annotateline(object):
 fctx = attr.ib()
 lineno = attr.ib(default=False)
+# Whether this annotation was the result of a skip-annotate.
+skip = attr.ib(default=False)
 
 def _annotatepair(parents, childfctx, child, skipchild, diffopts):
 r'''
@@ -1159,7 +1161,7 @@
 for bk in xrange(b1, b2):
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
-child[0][bk] = parent[0][ak]
+child[0][bk] = attr.evolve(parent[0][ak], 
skip=True)
 else:
 remaining[idx][1].append((a1, a2, b1, b2))
 
@@ -1170,7 +1172,7 @@
 for bk in xrange(b1, b2):
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
-child[0][bk] = parent[0][ak]
+child[0][bk] = attr.evolve(parent[0][ak], skip=True)
 return child
 
 class filectx(basefilectx):



To: sid0, #hg-reviewers
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
sid0 updated this revision to Diff 2329.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D899?vs=2320=2329

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

AFFECTED FILES
  mercurial/context.py
  tests/test-annotate.py

CHANGE DETAILS

diff --git a/tests/test-annotate.py b/tests/test-annotate.py
--- a/tests/test-annotate.py
+++ b/tests/test-annotate.py
@@ -80,20 +80,22 @@
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
+annotateline('old', 2, True),
+# note that this line was carried over from earlier so it is *not*
+# marked skipped
 annotateline('p2', 2),
-annotateline('p2', 2),
+annotateline('p2', 2, True),
 annotateline('p2', 3),
 ])
 
 childann = decorate(childdata, childfctx)
 childann = _annotatepair([p2ann, p1ann], childfctx, childann, True,
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
-annotateline('p1', 3),
-annotateline('p1', 3),
+annotateline('old', 2, True),
+annotateline('p1', 3, True),
+annotateline('p1', 3, True),
 annotateline('p2', 3),
 ])
 
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -,6 +,8 @@
 class annotateline(object):
 fctx = attr.ib()
 lineno = attr.ib(default=False)
+# Whether this annotation was the result of a skip-annotate.
+skip = attr.ib(default=False)
 
 def _annotatepair(parents, childfctx, child, skipchild, diffopts):
 r'''
@@ -1159,7 +1161,7 @@
 for bk in xrange(b1, b2):
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
-child[0][bk] = parent[0][ak]
+child[0][bk] = attr.evolve(parent[0][ak], 
skip=True)
 else:
 remaining[idx][1].append((a1, a2, b1, b2))
 
@@ -1170,7 +1172,7 @@
 for bk in xrange(b1, b2):
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
-child[0][bk] = parent[0][ak]
+child[0][bk] = attr.evolve(parent[0][ak], skip=True)
 return child
 
 class filectx(basefilectx):



To: sid0, #hg-reviewers
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
sid0 added inline comments.

INLINE COMMENTS

> sid0 wrote in context.py:1164-1165
> Good question! Not in this case, because a particular annotation can never go 
> from skip=True to skip=False. If we decide to overwrite the annotation 
> afterwards, the whole object is replaced, not just fctx and lineno.

Actually -- hmm, you're right, this is a bit risky to code changes in the 
future -- especially if the same object gets shared between skipped and 
not-skipped lines. I'll create a new one to be safe.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

To: sid0, #hg-reviewers
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
sid0 added a comment.


  I'll add a comment explaining that.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

To: sid0, #hg-reviewers
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
sid0 added inline comments.

INLINE COMMENTS

> indygreg wrote in context.py:1164-1165
> I see that we're copying a ref to the object instead of making an object 
> copy. When we had tuples, that was fine because tuples are immutable. But 
> with attr, instances can be modified.
> 
> Will this pose any problems?

Good question! Not in this case, because a particular annotation can never go 
from skip=True to skip=False. If we decide to overwrite the annotation 
afterwards, the whole object is replaced, not just fctx and lineno.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

To: sid0, #hg-reviewers
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread indygreg (Gregory Szorc) 
indygreg added a comment.


  Before I stamp this I'd like an answer to the mutability concerns.

INLINE COMMENTS

> context.py:1164-1165
>  ak = min(a1 + (bk - b1), a2 - 1)
>  child[0][bk] = parent[0][ak]
> +child[0][bk].skip = True
>  else:

I see that we're copying a ref to the object instead of making an object copy. 
When we had tuples, that was fine because tuples are immutable. But with attr, 
instances can be modified.

Will this pose any problems?

> context.py:1177
>  child[0][bk] = parent[0][ak]
> +child[0][bk].skip = True
>  return child

Ditto.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

To: sid0, #hg-reviewers
Cc: indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D899: annotate: track whether a particular annotation was the result of a skip

2017-10-02 Thread sid0 (Siddharth Agarwal) 
sid0 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We're going to expose this information in the UI in an upcoming patch.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D899

AFFECTED FILES
  mercurial/context.py
  tests/test-annotate.py

CHANGE DETAILS

diff --git a/tests/test-annotate.py b/tests/test-annotate.py
--- a/tests/test-annotate.py
+++ b/tests/test-annotate.py
@@ -80,20 +80,20 @@
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
-annotateline('p2', 2),
-annotateline('p2', 2),
+annotateline('old', 2, True),
+annotateline('p2', 2, True),
+annotateline('p2', 2, True),
 annotateline('p2', 3),
 ])
 
 childann = decorate(childdata, childfctx)
 childann = _annotatepair([p2ann, p1ann], childfctx, childann, True,
  diffopts)
 self.assertEqual(childann[0], [
 annotateline('old', 1),
-annotateline('old', 2),
-annotateline('p1', 3),
-annotateline('p1', 3),
+annotateline('old', 2, True),
+annotateline('p1', 3, True),
+annotateline('p1', 3, True),
 annotateline('p2', 3),
 ])
 
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -,6 +,8 @@
 class annotateline(object):
 fctx = attr.ib()
 lineno = attr.ib(default=False)
+# Whether this annotation was the result of a skip-annotate.
+skip = attr.ib(default=False)
 
 def _annotatepair(parents, childfctx, child, skipchild, diffopts):
 r'''
@@ -1160,6 +1162,7 @@
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
 child[0][bk] = parent[0][ak]
+child[0][bk].skip = True
 else:
 remaining[idx][1].append((a1, a2, b1, b2))
 
@@ -1171,6 +1174,7 @@
 if child[0][bk].fctx == childfctx:
 ak = min(a1 + (bk - b1), a2 - 1)
 child[0][bk] = parent[0][ak]
+child[0][bk].skip = True
 return child
 
 class filectx(basefilectx):



To: sid0, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel