Hopefully fix unicodes and indented pretty printing

[aaruni96]

Fixes #1561

[codecov]

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (c563d60) 87.17% compared to head (be44068) 86.98%.
Report is 27 commits behind head on master.

Files	Patch %	Lines
src/PrettyPrinting.jl	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1581      +/-   ##
==========================================
- Coverage   87.17%   86.98%   -0.20%     
==========================================
  Files         115      114       -1     
  Lines       29565    29686     +121     
==========================================
+ Hits        25773    25822      +49     
- Misses       3792     3864      +72     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[thofma]

Thanks. I would like to see a bit more tests, things with non-standard width like '⛵' and where it has to wrap a line.

[aaruni96]

Fixed the case where the spilled over line still had unicode.

Now looking into characters with non standard widths.

[lgoettgens]

Not getting weird errors from unicode chars getting ripped in half is already a great step IMO.
handling chars of different widths and still get visually a fixed with of x columns in the terminal is great to have, but I think much harder to implement. Thus I would be happy to merge as is (with a few more tests) and get everything else in a future PR.

[fingolfin]

I agree 100% with @lgoettgens . I have zero issues with printing involving double width chars right now, but I've run into errors due to multibyte single width chars in two completely independent situations by now.

[lgoettgens]

I think most of this could be written very nice and concise using the second dispatch (with UnitRange) of https://docs.julialang.org/en/v1/stdlib/Unicode/#Unicode.graphemes, but unfortunately this is only existent from julia 1.9. So maybe add a comment referencing this so we can rewrite once the compat bound is changed. Or we include the function for older versions ourselves

[aaruni96]

I've found a strange bug already, while trying to write a test. Please do not merge this in its current state.

[fingolfin]

I marked this as a "draft" PR for now :-)

[lgoettgens]

The code changes look very clean now after the refactoring. Thanks!

Whats left from the comments above: Can you add some tests using something with many combiners (like in #1581 (comment)) and something of non-standard width (like in #1581 (comment))?

[aaruni96]

No longer crashes when graphemes of non standard width are encountered, but currently does not account for it while calculating when to line break.

I will add tests including wide graphemes along with the patch that makes them print in line with other characters.

[fingolfin]

Just had a quick look. The println(io) bit must be changed. I think the lowercasefirst handling should also be changed then as we need to re-run tests anyway, and it simplifies the code. Your choice about the iterators

[fingolfin]

Tests look good but there are a few simple optimizations to be had, see the code suggestions. Other than that I am OK with merging this. Feel free to come over to my office to discuss any of these.

[fingolfin]

Thanks @aaruni96 !!!