Re: [PATCH] histedit: fix diff colors
pushed on stable though heptapod. Thanks On 3/15/23 19:55, Jordi Gutiérrez Hermoso wrote: # HG changeset patch # User Jordi Gutiérrez Hermoso # Date 1666906442 14400 # Thu Oct 27 17:34:02 2022 -0400 # Node ID 28cad0a7eb26a3bb0edd4623d1ec1c9169eb49e2 # Parent dd42156b6441f6b8356100b4228fa16fbf95f669 histedit: fix diff colors The problem here is that indexing a bytestring gives you integers, not chars, so the comparison to b'+' ends up being wrong. We don't really have a way to test curses output, so no tests to verify the correctness of this behaviour. diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -1427,11 +1427,11 @@ pgup/K: move patch up, pgdn/J: move patc for y in range(0, length): line = output[y] if diffcolors: -if line and line[0] == b'+': +if line.startswith(b'+'): win.addstr( y, 0, line, curses.color_pair(COLOR_DIFF_ADD_LINE) ) -elif line and line[0] == b'-': +elif line.startswith(b'-'): win.addstr( y, 0, line, curses.color_pair(COLOR_DIFF_DEL_LINE) ) ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: fix diff colors
Patch LGTM > On Mar 15, 2023, at 14:55, Jordi Gutiérrez Hermoso wrote: > > # HG changeset patch > # User Jordi Gutiérrez Hermoso > # Date 1666906442 14400 > # Thu Oct 27 17:34:02 2022 -0400 > # Node ID 28cad0a7eb26a3bb0edd4623d1ec1c9169eb49e2 > # Parent dd42156b6441f6b8356100b4228fa16fbf95f669 > histedit: fix diff colors > > The problem here is that indexing a bytestring gives you integers, not > chars, so the comparison to b'+' ends up being wrong. > > We don't really have a way to test curses output, so no tests to > verify the correctness of this behaviour. > > diff --git a/hgext/histedit.py b/hgext/histedit.py > --- a/hgext/histedit.py > +++ b/hgext/histedit.py > @@ -1427,11 +1427,11 @@ pgup/K: move patch up, pgdn/J: move patc > for y in range(0, length): > line = output[y] > if diffcolors: > -if line and line[0] == b'+': > +if line.startswith(b'+'): > win.addstr( > y, 0, line, curses.color_pair(COLOR_DIFF_ADD_LINE) > ) > -elif line and line[0] == b'-': > +elif line.startswith(b'-'): > win.addstr( > y, 0, line, curses.color_pair(COLOR_DIFF_DEL_LINE) > ) > ___ > Mercurial-devel mailing list > Mercurial-devel@lists.mercurial-scm.org > https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] histedit: fix diff colors
# HG changeset patch # User Jordi Gutiérrez Hermoso # Date 1666906442 14400 # Thu Oct 27 17:34:02 2022 -0400 # Node ID 28cad0a7eb26a3bb0edd4623d1ec1c9169eb49e2 # Parent dd42156b6441f6b8356100b4228fa16fbf95f669 histedit: fix diff colors The problem here is that indexing a bytestring gives you integers, not chars, so the comparison to b'+' ends up being wrong. We don't really have a way to test curses output, so no tests to verify the correctness of this behaviour. diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -1427,11 +1427,11 @@ pgup/K: move patch up, pgdn/J: move patc for y in range(0, length): line = output[y] if diffcolors: -if line and line[0] == b'+': +if line.startswith(b'+'): win.addstr( y, 0, line, curses.color_pair(COLOR_DIFF_ADD_LINE) ) -elif line and line[0] == b'-': +elif line.startswith(b'-'): win.addstr( y, 0, line, curses.color_pair(COLOR_DIFF_DEL_LINE) ) ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: fix diff colors
On Thu, 2022-10-27 at 17:37 -0400, Jordi Gutiérrez Hermoso wrote: > On Tue, 2022-10-25 at 21:48 +0200, Joerg Sonnenberger wrote: > > Do we still need the "line and" part of the check? > > Good call, no longer necessary. Will fix in v2, along with a test. Wait, do we have a way to test curses output? ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: fix diff colors
On Tue, 2022-10-25 at 21:48 +0200, Joerg Sonnenberger wrote: > Do we still need the "line and" part of the check? Good call, no longer necessary. Will fix in v2, along with a test. ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: fix diff colors
Am Mon, Oct 24, 2022 at 05:05:18PM -0400 schrieb Jordi Gutiérrez Hermoso: > I don't know what was up here, but seems like checking for the first > byte isn't the same thing as "startswith", so the colours in chistedit > diffs were broken. Can you also please build a test case for this? Joerg ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: fix diff colors
On Mon, Oct 24, 2022 at 05:05:18PM -0400, Jordi Gutiérrez Hermoso wrote: > diff --git a/hgext/histedit.py b/hgext/histedit.py > --- a/hgext/histedit.py > +++ b/hgext/histedit.py > @@ -1428,11 +1428,11 @@ pgup/K: move patch up, pgdn/J: move patc > for y in range(0, length): > line = output[y] > if diffcolors: > -if line and line[0] == b'+': > +if line and line.startswith(b'+'): > win.addstr( > y, 0, line, curses.color_pair(COLOR_DIFF_ADD_LINE) > ) > -elif line and line[0] == b'-': > +elif line and line.startswith(b'-'): > win.addstr( > y, 0, line, curses.color_pair(COLOR_DIFF_DEL_LINE) > ) Do we still need the "line and" part of the check? Joerg ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: fix diff colors
This patch looks obviously-correct to me and should be pushed. What’s going on here is that byte_string[index] returns an int, not a byte, so the .startswith() is a strict correctness improvement. Thanks! > On Oct 24, 2022, at 5:05 PM, Jordi Gutiérrez Hermoso > wrote: > > # HG changeset patch > # User Jordi Gutiérrez Hermoso > # Date 145456 14400 > # Mon Oct 24 17:04:16 2022 -0400 > # Node ID 5287d8898362935f06b482b236ca6ba948c6850d > # Parent f68d285158b21d7d13c2a70c8db85e83a83ee392 > histedit: fix diff colors > > I don't know what was up here, but seems like checking for the first > byte isn't the same thing as "startswith", so the colours in chistedit > diffs were broken. > > diff --git a/hgext/histedit.py b/hgext/histedit.py > --- a/hgext/histedit.py > +++ b/hgext/histedit.py > @@ -1428,11 +1428,11 @@ pgup/K: move patch up, pgdn/J: move patc > for y in range(0, length): > line = output[y] > if diffcolors: > -if line and line[0] == b'+': > +if line and line.startswith(b'+'): > win.addstr( > y, 0, line, curses.color_pair(COLOR_DIFF_ADD_LINE) > ) > -elif line and line[0] == b'-': > +elif line and line.startswith(b'-'): > win.addstr( > y, 0, line, curses.color_pair(COLOR_DIFF_DEL_LINE) > ) > ___ > Mercurial-devel mailing list > Mercurial-devel@lists.mercurial-scm.org > https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] histedit: fix diff colors
# HG changeset patch # User Jordi Gutiérrez Hermoso # Date 145456 14400 # Mon Oct 24 17:04:16 2022 -0400 # Node ID 5287d8898362935f06b482b236ca6ba948c6850d # Parent f68d285158b21d7d13c2a70c8db85e83a83ee392 histedit: fix diff colors I don't know what was up here, but seems like checking for the first byte isn't the same thing as "startswith", so the colours in chistedit diffs were broken. diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -1428,11 +1428,11 @@ pgup/K: move patch up, pgdn/J: move patc for y in range(0, length): line = output[y] if diffcolors: -if line and line[0] == b'+': +if line and line.startswith(b'+'): win.addstr( y, 0, line, curses.color_pair(COLOR_DIFF_ADD_LINE) ) -elif line and line[0] == b'-': +elif line and line.startswith(b'-'): win.addstr( y, 0, line, curses.color_pair(COLOR_DIFF_DEL_LINE) ) ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
