feat: multiple characters for rule.py

[hedyhli]

Type of changes

• New feature

(Removing these other options so progress bar of pr can show proper progress)

Checklist

• I've run the latest black with default args on new code.
• I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
• I've added tests for new code.
• I accept that @willmcgugan may be pedantic in the code review.

---

• Take care of super long strings for characters param
• support cjk characters
• support emoji
• update rule function in console.py

Description

Resolves #204

Add support for multiple characters param in printing a horizontal rule, supported emoji sizing and cjk printing. Added tests.

[codecov]

Codecov Report

Merging #207 into master will increase coverage by 0.56%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
+ Coverage   97.85%   98.42%   +0.56%     
==========================================
  Files          50       50              
  Lines        3783     3930     +147     
==========================================
+ Hits         3702     3868     +166     
+ Misses         81       62      -19     

Flag	Coverage Δ
#unittests	98.42% <91.66%> (+0.56%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
rich/console.py	99.09% <66.66%> (-0.91%)	⬇️
rich/rule.py	97.72% <95.23%> (-2.28%)	⬇️
rich/text.py	99.78% <0.00%> (-0.22%)	⬇️
rich/cells.py	100.00% <0.00%> (ø)
rich/pretty.py	100.00% <0.00%> (ø)
rich/progress.py	90.00% <0.00%> (ø)
rich/default_styles.py	100.00% <0.00%> (ø)
rich/_ratio.py	100.00% <0.00%> (+2.70%)	⬆️
rich/containers.py	98.83% <0.00%> (+4.65%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1501671...4391f61. Read the comment docs.

[hedyhli]

I was able to get the cell sizes of emoji, but had some trouble with printing emoji,

(they overlap each other)
problem seems to happen around here
I think. Also for Chinese characters, this ends up happening --- 你好！\n ---\n note the extra \n after title

[willmcgugan]

There's not much you can do re overlapping emoji I'm afraid, it depends on the terminal. Try other emoji / terminals to see if there is a difference.

Not sure where your extra new line is coming from.

[hedyhli]

I've just tried the emoji rule in my own terminal and there aren't any overlaps anymore. But the last emoji gets broken onto the next line, because of the 2 cell length it has (same goes to Chinese characters for characters param). I don't think it will be much of a problem as long as people don't get a too unexpected result when they try to do a rule with emoji. Because using emoji is a bit uncommon in my opinion. I will investigate in cjk character rules immediatly.

[willmcgugan]

Ah, I see why there was an extra newline. The text was longer than the width of the terminal and it wrapped.

have a look at cells.set_cell_size. It will set the cell width of a string, and handles the case of halving an emoji/cjk by replacing it with a space.

I do think it's worthwhile to handle these edge cases. If only so I have less issues to respond to.

[hedyhli]

I have fixed the extra newline, do you think we should add tests for emoji? Since you said the overlapping can differ due to different terminals I haven’t added them yet

I’m also not quite sure about the CONTRIBUTORS.md, people have contributed but no one has added them in it yet

[willmcgugan]

I think we'll need a few changes.

Most of the stuff in Rich has turned out to be trickier than you might think. Especially when cell length is taken in to consideration!

[willmcgugan]

Thanks, I'll have a look at your changes soon. Working on 5.1.0 this weekend.

[willmcgugan]

@hedythedev This looks good. All we need is to add 'character' back in to the interface and we should be good to merge.

[hedyhli]

The only problem now, is that I did not add tests for line 31 (as the coverage states) because then you will also have to remove the test when the next major bump takes place, so the coverage for rule will be 98%. If you think it’s fine I will add them for both rule and console

[willmcgugan]

One option would be to do this:

characters = character or characters

Without the branch, coverage won't see that as a miss. But you could argue that's a hack to avoid a test.

tbh I don't think it will make much difference either way. Happy to merge. Thanks for your contribution.

I have branch I've been working on and I should be able to get the new rule in the next minor version, hopefully today or tomorrow.

[hedyhli]

Would it be necessary to add me to CONTRIBUTORS.md? I think I forgot to do that

[willmcgugan]

Yes please. I’d like to keep track of contributors there.