Patchwork D3665: graph: improve graph output by using Unicode characters

login
register
mail settings
Submitter Yuya Nishihara
Date May 30, 2018, 12:30 p.m.
Message ID <20180530213002.cb78235479538fdce8fbe7bf@tcha.org>
Download mbox | patch
Permalink /patch/31904/
State New
Headers show

Comments

Yuya Nishihara - May 30, 2018, 12:30 p.m.
>   A proper warning is now issued when the encoding is not UTF-8, or when East-Asian ambiguous characters will be rendered as wide characters.
>   The tests have been updated to check these warnings.
>   Yuya, I believe this should address everything we discussed. Let me know if you see a need for any further changes.

Yeah, the changes look good to me, thanks. Sean and others, can you review
and queue this if it seems good to go into the core extension? I'm 0 on this
feature.

A couple more nits:

```

> A terminal with UTF-8 support and a monospace Unicode font are required.

Strictly speaking, the font doesn't matter. It's up to terminal emulator
how to lay out each character.

> from mercurial import encoding
> from mercurial import extensions
> from mercurial import templatekw
> from mercurial import graphmod

We prefer `from mercurial import (
    foo,
    bar,
    baz,
)`.
phabricator - May 30, 2018, 12:31 p.m.
yuja added a comment.


  >   A proper warning is now issued when the encoding is not UTF-8, or when East-Asian ambiguous characters will be rendered as wide characters.
  >   The tests have been updated to check these warnings.
  >   Yuya, I believe this should address everything we discussed. Let me know if you see a need for any further changes.
  
  Yeah, the changes look good to me, thanks. Sean and others, can you review
  and queue this if it seems good to go into the core extension? I'm 0 on this
  feature.
  
  A couple more nits:
  
    --- tests/test-check-commit.t
    +++ tests/test-check-commit.t.err
    @@ -25,3 +25,14 @@
       >     fi
       >   done
       > fi
    +  Revision 2c4617bda693 does not comply with rules
    +  ------------------------------------------------------
    +  167: adds double empty line
    +   +
    +  235: adds double empty line
    +   +
    +  1209: adds double empty line
    +   +
    +  1403: adds double empty line
    +   +
    +  
    
    ERROR: test-check-commit.t output changed
    !...
    --- tests/test-check-pylint.t
    +++ tests/test-check-pylint.t.err
    @@ -19,3 +19,22 @@
       ------------------------------------ (?)
       Your code has been rated at 10.00/10 (?)
        (?)
    +  Using config file $TESTTMP/fakerc
    +  ************* Module hgext.beautifygraph
    +  C: 38,29: More than one statement on a single line (multiple-statements)
    +  C: 39,29: More than one statement on a single line (multiple-statements)
    +  C: 40,29: More than one statement on a single line (multiple-statements)
    +  C: 41,29: More than one statement on a single line (multiple-statements)
    +  C: 42,29: More than one statement on a single line (multiple-statements)
    +  C: 43,29: More than one statement on a single line (multiple-statements)
    +  C: 44,29: More than one statement on a single line (multiple-statements)
    +  C: 60,24: More than one statement on a single line (multiple-statements)
    +  C: 61,24: More than one statement on a single line (multiple-statements)
    +  C: 62,24: More than one statement on a single line (multiple-statements)
    +  C: 63,24: More than one statement on a single line (multiple-statements)
    +  C: 64,24: More than one statement on a single line (multiple-statements)
  
  
  
  > 1. beautifygraph.py - improve graph output by using Unicode characters
  
  Can you add a copyright line and GPL placeholder?
  
  > A terminal with UTF-8 support and a monospace Unicode font are required.
  
  Strictly speaking, the font doesn't matter. It's up to terminal emulator
  how to lay out each character.
  
  > from mercurial import encoding
  >  from mercurial import extensions
  >  from mercurial import templatekw
  >  from mercurial import graphmod
  
  We prefer `from mercurial import (
  
    foo,
    bar,
    baz,
  
  )`.

REPOSITORY
  rHG Mercurial

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

To: johnstiles, #hg-reviewers
Cc: smf, yuja, mercurial-devel

Patch

--- tests/test-check-commit.t
+++ tests/test-check-commit.t.err
@@ -25,3 +25,14 @@ 
   >     fi
   >   done
   > fi
+  Revision 2c4617bda693 does not comply with rules
+  ------------------------------------------------------
+  167: adds double empty line
+   +
+  235: adds double empty line
+   +
+  1209: adds double empty line
+   +
+  1403: adds double empty line
+   +
+  

ERROR: test-check-commit.t output changed
!...
--- tests/test-check-pylint.t
+++ tests/test-check-pylint.t.err
@@ -19,3 +19,22 @@ 
   ------------------------------------ (?)
   Your code has been rated at 10.00/10 (?)
    (?)
+  Using config file $TESTTMP/fakerc
+  ************* Module hgext.beautifygraph
+  C: 38,29: More than one statement on a single line (multiple-statements)
+  C: 39,29: More than one statement on a single line (multiple-statements)
+  C: 40,29: More than one statement on a single line (multiple-statements)
+  C: 41,29: More than one statement on a single line (multiple-statements)
+  C: 42,29: More than one statement on a single line (multiple-statements)
+  C: 43,29: More than one statement on a single line (multiple-statements)
+  C: 44,29: More than one statement on a single line (multiple-statements)
+  C: 60,24: More than one statement on a single line (multiple-statements)
+  C: 61,24: More than one statement on a single line (multiple-statements)
+  C: 62,24: More than one statement on a single line (multiple-statements)
+  C: 63,24: More than one statement on a single line (multiple-statements)
+  C: 64,24: More than one statement on a single line (multiple-statements)
```

> # beautifygraph.py - improve graph output by using Unicode characters

Can you add a copyright line and GPL placeholder?