Skip to content

Make wcwidth Python library into a soft dependency in the source code#282

Merged
mulkieran merged 3 commits into
stratis-storage:masterfrom
mulkieran:master-wcwidth
May 30, 2019
Merged

Make wcwidth Python library into a soft dependency in the source code#282
mulkieran merged 3 commits into
stratis-storage:masterfrom
mulkieran:master-wcwidth

Conversation

@mulkieran

@mulkieran mulkieran commented May 30, 2019

Copy link
Copy Markdown
Member

Fall back to more incorrect alignment of columns containing non-standard width characters if wcwidth Python library is unavailable.

It would be wrong for the whole program to fail to run because it might not align its output properly in columns if it did run.

Make some additional changes congruent with this change.

Do not change the dependency requirement for wcwidth in setup.py. setup.py is what pip relies on, and wcwidth is available on PyPi, so that is the correct thing to do.

@mulkieran mulkieran self-assigned this May 30, 2019
@mulkieran

mulkieran commented May 30, 2019

Copy link
Copy Markdown
Member Author

With wcwidth installed. There's a bit of misalignment here, due to some Firefox or markup behavior, but it's not there on my terminal.

$ python3 src/stratis_cli/_actions/_formatting.py
Example table...
Pool Name    Name  Used     Created            Device                       
yes_you_can  ☺     546 MiB  Oct 05 2018 16:24  /dev/stratis/yes_you_can/☺   
yes_you_can  漢字  546 MiB  Oct 10 2018 09:37  /dev/stratis/yes_you_can/漢字

Example table...
Pool Nåme  Nåme  Used     Created            Device                 UUID                            
unicode    e     546 MiB  Feb 07 2019 15:33  /stratis/unicode/e     3bf22806a6df4660aa527d646209595f
unicode    eeee  546 MiB  Feb 07 2019 15:33  /stratis/unicode/eeee  17101e39e72e423c90d8be5cb37c055b
unicodé    é     546 MiB  Feb 07 2019 15:33  /stratis/unicodé/é     0c2caf641dde41beb40bed6911f75c74
unicodé    éééé  546 MiB  Feb 07 2019 15:33  /stratis/unicodé/éééé  4ecacb15fb64453191d7da731c5f1601

Without wcwidth installed. Here there is lots of misalignment.

$ python3 src/stratis_cli/_actions/_formatting.py
Example table...
Pool Name    Name  Used     Created            Device                     
yes_you_can  ☺     546 MiB  Oct 05 2018 16:24  /dev/stratis/yes_you_can/☺ 
yes_you_can  漢字    546 MiB  Oct 10 2018 09:37  /dev/stratis/yes_you_can/漢字

Example table...
Pool Nåme  Nåme     Used     Created            Device                      UUID                            
unicode     e         546 MiB  Feb 07 2019 15:33  /stratis/unicode/e          3bf22806a6df4660aa527d646209595f
unicode     eeee      546 MiB  Feb 07 2019 15:33  /stratis/unicode/eeee       17101e39e72e423c90d8be5cb37c055b
unicodé    é        546 MiB  Feb 07 2019 15:33  /stratis/unicodé/é        0c2caf641dde41beb40bed6911f75c74
unicodé    éééé  546 MiB  Feb 07 2019 15:33  /stratis/unicodé/éééé  4ecacb15fb64453191d7da731c5f1601

@mulkieran mulkieran changed the title Master wcwidth Make wcwidth Python library into a soft dependency May 30, 2019
@mulkieran mulkieran changed the title Make wcwidth Python library into a soft dependency Make wcwidth Python library into a soft dependency in the source code May 30, 2019
@mulkieran mulkieran requested review from bgurney-rh and drckeefe May 30, 2019 14:25
mulkieran added 3 commits May 30, 2019 10:52
It may not be properly installed on some distributions.

Signed-off-by: mulhern <amulhern@redhat.com>
Also, add a note about how defaulting to len changes the behavior.

Signed-off-by: mulhern <amulhern@redhat.com>
In most of the code, "width" means number of cells, "length" means number
of chars. It's easier to read if the naming is consistent here as well.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran

Copy link
Copy Markdown
Member Author

rebased.

@mulkieran mulkieran merged commit c8351ad into stratis-storage:master May 30, 2019
@mulkieran mulkieran deleted the master-wcwidth branch May 30, 2019 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants