Fix height and filler when wrapped at the middle of wide characters#799
Fix height and filler when wrapped at the middle of wide characters#799kojiishi wants to merge 1 commit into
Conversation
kojiishi
force-pushed
the
cjk
branch
9 times, most recently
from
713282e to
a48771a
Compare
|
Updated the commit message to be more descriptive. |
| } | ||
| } | ||
| } | ||
| let effective_width = (num_lines - 1) * width + column; |
There was a problem hiding this comment.
I'm not sure effective_width is a good name for this? At least it seems confusing that we're multiplying num_lines with a width and also calling the result a width.
There was a problem hiding this comment.
Renamed to width_sum. How about this?
There was a problem hiding this comment.
I got another idea; I made it last_line_width, and modified draw_to_term accordingly. This saves some CPU cycles with unicode-width, adds some cycles when only height is needed without unicode-width, but subtle. Does this look clearer?
| continue; | ||
| } | ||
| for ch in substr.chars() { | ||
| let ch_width = unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0); |
There was a problem hiding this comment.
Please import UnicodeWidthChar at the top of the module. I think the unwrap_or(0) here is a little weird. I think we can do let Some(ch_width) = .. else { continue }; instead?
There was a problem hiding this comment.
Done, and thank you for the good suggestion again, done.
| } | ||
|
|
||
| #[cfg(not(feature = "unicode-width"))] | ||
| fn wrapped_height_and_effective_width(&self, width: usize) -> (VisualLines, usize) { |
There was a problem hiding this comment.
I find this name a little verbose. Maybe something like metrics? Could even introduce a Metrics struct for the return type, maybe.
There was a problem hiding this comment.
Thank you for the suggestion, I like that. Introduced a Metrics struct.
|
What are your uses for? Is indicatif being used in Chromium? |
No, this is for my personal project https://github.com/kojiishi/compare-dir, half for own use and half for learning Rust. |
kojiishi
force-pushed
the
cjk
branch
2 times, most recently
from
35f7856 to
37599fb
Compare
When a wide character such as CJK appears at the end of wrap with only 1 column available, the line wraps before the character, leaving an empty column. This patch fixes `wrapped_height` to take them into account, by computing each wrap instead of dividing by the width. Fixes console-rs#798.
2 participants
When a wide character such as CJK appears at the end of wrap with only 1 column available, the line wraps before the character, leaving an empty column.
This patch fixes
wrapped_heightto take them into account, by computing each wrap instead of dividing by the width.Fixes #798.