Patchwork annotate: calculate line count correctly

login
register
mail settings
Submitter Jun Wu
Date Oct. 1, 2016, 1:20 p.m.
Message ID <dfd539e1e012e2fa78c0.1475328005@x1c>
Download mbox | patch
Permalink /patch/16816/
State Accepted
Headers show

Comments

Jun Wu - Oct. 1, 2016, 1:20 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1475327938 -3600
#      Sat Oct 01 14:18:58 2016 +0100
# Node ID dfd539e1e012e2fa78c0635e0e4bc993f7bbd89e
# Parent  3741a8f86e88702595c29f8ed824a28da0cfa961
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r dfd539e1e012
# EXP-Topic extensions.debug
annotate: calculate line count correctly

Before this patch, the "lines" function inside "annotate" returns 1 for
empty text (''). This patch makes it 0. Because the function should match
mdiff.splitnewlines (used by mdiff.allblocks), or s.splitlines (used at the
end of the "annotate" method). Both len(mdiff.splitnewlines('')) and
len(''.splitlines(True)) are 0.

This issue was discovered while testing fastannotate [1].

I could not find a test case to reveal this issue. However in theory this
could reduce memory usage a little bit, and avoids surprises when people
are touching this area in the future.

[1]: https://bitbucket.org/facebook/hg-experimental/commits/525b3b98e93a
Jun Wu - Oct. 1, 2016, 1:34 p.m.
Excerpts from Jun Wu's message of 2016-10-01 14:20:05 +0100:
> # EXP-Topic extensions.debug

Sorry. I didn't realize the topic name. I have topics extension enabled but
didn't run the topic command. And it reuses the topic the parent has.

This is also one of the reasons I dislike the current topic design - I think
I should be able to commit on top of others' commits (in this case, @)
without thinking about topic names.
Augie Fackler - Oct. 2, 2016, 9:59 p.m.
> On Oct 1, 2016, at 9:20 AM, Jun Wu <quark@fb.com> wrote:
> 
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1475327938 -3600
> #      Sat Oct 01 14:18:58 2016 +0100
> # Node ID dfd539e1e012e2fa78c0635e0e4bc993f7bbd89e
> # Parent  3741a8f86e88702595c29f8ed824a28da0cfa961
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r dfd539e1e012
> # EXP-Topic extensions.debug
> annotate: calculate line count correctly

queued this, thanks

> 
> Before this patch, the "lines" function inside "annotate" returns 1 for
> empty text (''). This patch makes it 0. Because the function should match
> mdiff.splitnewlines (used by mdiff.allblocks), or s.splitlines (used at the
> end of the "annotate" method). Both len(mdiff.splitnewlines('')) and
> len(''.splitlines(True)) are 0.
> 
> This issue was discovered while testing fastannotate [1].
> 
> I could not find a test case to reveal this issue. However in theory this
> could reduce memory usage a little bit, and avoids surprises when people
> are touching this area in the future.
> 
> [1]: https://bitbucket.org/facebook/hg-experimental/commits/525b3b98e93a
> 
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -931,5 +931,5 @@ class basefilectx(object):
>             if text.endswith("\n"):
>                 return text.count("\n")
> -            return text.count("\n") + 1
> +            return text.count("\n") + int(bool(text))
> 
>         if linenumber:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -931,5 +931,5 @@  class basefilectx(object):
             if text.endswith("\n"):
                 return text.count("\n")
-            return text.count("\n") + 1
+            return text.count("\n") + int(bool(text))
 
         if linenumber: