This is an archive of the discontinued Mercurial Phabricator instance.

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

Authored by sid0 on Oct 2 2017, 5:35 AM.



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

Diff Detail

rHG Mercurial
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sid0 created this revision.Oct 2 2017, 5:35 AM

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


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?



sid0 added inline comments.Oct 2 2017, 10:56 AM

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.

sid0 added a comment.Oct 2 2017, 10:57 AM

I'll add a comment explaining that.

sid0 added inline comments.Oct 2 2017, 10:59 AM

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.

sid0 updated this revision to Diff 2329.Oct 2 2017, 11:05 AM
sid0 updated this revision to Diff 2331.Oct 2 2017, 11:08 AM
sid0 added a comment.Oct 2 2017, 11:09 AM

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)

indygreg accepted this revision.Oct 2 2017, 1:14 PM

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

This revision is now accepted and ready to land.Oct 2 2017, 1:14 PM
This revision was automatically updated to reflect the committed changes.