Patchwork crecord: fixes the formatting of the select status in the status line

login
register
mail settings
Submitter Filip Filmar
Date Aug. 13, 2017, 8:04 a.m.
Message ID <4a5fd64feb902da51e7e.1502611492@fmil-glaptop4.roam.corp.google.com>
Download mbox | patch
Permalink /patch/22950/
State Accepted
Headers show

Comments

Filip Filmar - Aug. 13, 2017, 8:04 a.m.
# HG changeset patch
# User Filip Filmar <filmil@gmail.com>
# Date 1502608633 25200
#      Sun Aug 13 00:17:13 2017 -0700
# Node ID 4a5fd64feb902da51e7eb5759f4c1d3966186726
# Parent  03039ff3082b970e70f582e998a7287d1600e912
crecord: fixes the formatting of the select status in the status line

The status line in the crecord has the "space" status field which has variable
length depending on the length of the status label in the language of choice.
In English, the status labels are "space: deselect" and "space:select".  The
"deselect" label is 2 glyphs longer.  This makes the terminal output jump
around if the terminal width is just right so that the shorter label makes
the status line 1 line long, and the longer label makes it 2 lines long.

This patch formats the selected status into a fixed-width field.  The field
width is the maximum of the lengths of the two possible labels, to account for
differing translations and label lengths.  This should make the label behavior
uniform across localizations.

There does not seem to be a test for crecord, so I verified the change manually
with a local build of 'hg'.
Augie Fackler - Aug. 15, 2017, 7:54 p.m.
On Sun, Aug 13, 2017 at 01:04:52AM -0700, Filip Filmar wrote:
> # HG changeset patch
> # User Filip Filmar <filmil@gmail.com>
> # Date 1502608633 25200
> #      Sun Aug 13 00:17:13 2017 -0700
> # Node ID 4a5fd64feb902da51e7eb5759f4c1d3966186726
> # Parent  03039ff3082b970e70f582e998a7287d1600e912
> crecord: fixes the formatting of the select status in the status line

queued, thanks
Jun Wu - Aug. 15, 2017, 11:51 p.m.
len(text) does not take wide characters into consideration. So it does not
work with all localizations in theory. Maybe we can just add spaces here and
rely on translators to align them.

Excerpts from Filip Filmar's message of 2017-08-13 01:04:52 -0700:
> # HG changeset patch
> # User Filip Filmar <filmil@gmail.com>
> # Date 1502608633 25200
> #      Sun Aug 13 00:17:13 2017 -0700
> # Node ID 4a5fd64feb902da51e7eb5759f4c1d3966186726
> # Parent  03039ff3082b970e70f582e998a7287d1600e912
> crecord: fixes the formatting of the select status in the status line
> 
> The status line in the crecord has the "space" status field which has variable
> length depending on the length of the status label in the language of choice.
> In English, the status labels are "space: deselect" and "space:select".  The
> "deselect" label is 2 glyphs longer.  This makes the terminal output jump
> around if the terminal width is just right so that the shorter label makes
> the status line 1 line long, and the longer label makes it 2 lines long.
> 
> This patch formats the selected status into a fixed-width field.  The field
> width is the maximum of the lengths of the two possible labels, to account for
> differing translations and label lengths.  This should make the label behavior
> uniform across localizations.
> 
> There does not seem to be a test for crecord, so I verified the change manually
> with a local build of 'hg'.
> 
> diff -r 03039ff3082b -r 4a5fd64feb90 mercurial/crecord.py
> --- a/mercurial/crecord.py    Tue Aug 01 17:53:48 2017 +0200
> +++ b/mercurial/crecord.py    Sun Aug 13 00:17:13 2017 -0700
> @@ -1010,6 +1010,13 @@
>      def _getstatuslinesegments(self):
>          """-> [str]. return segments"""
>          selected = self.currentselecteditem.applied
> +        spaceselect = _('space: select')
> +        spacedeselect = _('space: deselect')
> +        # Format the selected label into a place as long as the longer of the
> +        # two possible labels.  This may vary by language.
> +        spacelen = max(len(spaceselect), len(spacedeselect))
> +        selectedlabel = '%-*s' % (spacelen,
> +                                  spacedeselect if selected else spaceselect)
>          segments = [
>              _headermessages[self.operation],
>              '-',
> @@ -1017,7 +1024,7 @@
>              _('c: confirm'),
>              _('q: abort'),
>              _('arrow keys: move/expand/collapse'),
> -            _('space: deselect') if selected else _('space: select'),
> +            selectedlabel,
>              _('?: help'),
>          ]
>          return segments
Yuya Nishihara - Aug. 16, 2017, 2:55 a.m.
On Tue, 15 Aug 2017 16:51:54 -0700, Jun Wu wrote:
> len(text) does not take wide characters into consideration. So it does not
> work with all localizations in theory. Maybe we can just add spaces here and
> rely on translators to align them.

encoding.colwidth() could be used.
Jun Wu - Aug. 16, 2017, 4:21 p.m.
Excerpts from Yuya Nishihara's message of 2017-08-16 11:55:00 +0900:
> encoding.colwidth() could be used.

Nice to know! I'll send a follow-up.

Patch

diff -r 03039ff3082b -r 4a5fd64feb90 mercurial/crecord.py
--- a/mercurial/crecord.py	Tue Aug 01 17:53:48 2017 +0200
+++ b/mercurial/crecord.py	Sun Aug 13 00:17:13 2017 -0700
@@ -1010,6 +1010,13 @@ 
     def _getstatuslinesegments(self):
         """-> [str]. return segments"""
         selected = self.currentselecteditem.applied
+        spaceselect = _('space: select')
+        spacedeselect = _('space: deselect')
+        # Format the selected label into a place as long as the longer of the
+        # two possible labels.  This may vary by language.
+        spacelen = max(len(spaceselect), len(spacedeselect))
+        selectedlabel = '%-*s' % (spacelen,
+                                  spacedeselect if selected else spaceselect)
         segments = [
             _headermessages[self.operation],
             '-',
@@ -1017,7 +1024,7 @@ 
             _('c: confirm'),
             _('q: abort'),
             _('arrow keys: move/expand/collapse'),
-            _('space: deselect') if selected else _('space: select'),
+            selectedlabel,
             _('?: help'),
         ]
         return segments