Fix: Several Wordwrap issues - Indent, HangingIndent, international character widths #7714
Fix: Several Wordwrap issues - Indent, HangingIndent, international character widths #7714vadi2 merged 47 commits intoMudlet:developmentfrom
Conversation
…e to wrap and considered characters of width greater than one as valid linebreak points.
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
… into wrap-overhaul
…ndent = 0 explicit with bool firstLineOfParagraph
…er::appendLine and a subsequent wrap
…e where append is called on an empty buffer.
…pended text if the append is just going on a newline.
…ffer and TTextEdit
…s widths and linebreaks, make TBuffer::wrap to use that.
…ne cases & accept wrap points in the middle of a string of spaces)
…dth and move TTextProperties functions into the graphemeInfo namespace.
… into wrap-overhaul
…wrapLine and adjust all external calls to use the unified wrapLine method.
|
Perfect! Let's get this reviewed... |
|
Great work! Tested all of the related issues except the foreign language ones. The only one I couldn't get working is #5936 using the provided use case. It doesn't seem to wrap at all in a miniconsole? Can someone test/prove me wrong? |
You are correct, AFAIK miniconsoles don't automatically wrap based on their width. This PR fixes the underlying issue, but the code to demonstrate it needs to be modified to (first line, add |
vadi2
left a comment
There was a problem hiding this comment.
Looks great to me, thanks so much for your work!
Just had one trivial point of feedback.
| const TChar styling(fgColor, bgColor, | ||
| (mEchoingText ? (TChar::Echo | (flags & TChar::TestMask)) | ||
| : (flags & TChar::TestMask))); | ||
| buffer.back().push_back(styling); |
There was a problem hiding this comment.
Copied this styling push in from the original append method. I think this should cause errors as the buffer will have a value whereas the lineBuffer is empty but I've copied it over anyway out of fear of deleting things I don't understand.
There was a problem hiding this comment.
Well, I'm not entirely sure what is going on here - but I would point out that normally there is a one-to-one relationship between the TChars in the buffer and the QChars in the lineBuffer except for empty lines which do have a TChar but don't have any QChars for them - I can't recall the exact details but I suspect that this is so that the formatting persists from one line to the next unless it gets changed by an <SCR> sequence (I think most MUDs do a <SGR>0m at the start of each line to do that!).
…7923) <!-- Keep the title short & concise so anyone non-technical can understand it, the title appears in PTB changelogs --> #### Brief overview of PR changes/additions Added a check for whether new lines were added from trigger processing/wrapping and if the last line in the buffer is blank. If new lines were added and the last line in the buffer is empty, TBuffer::translateToPlainText will skip appending an additional blank line. #### Motivation for adding to Mudlet A discord user mentioned the wrap-overhaul causing their echoes to behave differently. #### Other info (issues closed, discussion etc) 4.19.1:  After PR #7714:  After this PR: 
getWrapInfo discarded all wrap info when totalWidth <= mWrapAt, including newline-triggered splits. Track hasNewline and skip the early return when newlines are present. Add C++ functional tests for insertText with newlines and echo with newlines in trigger mode. Add Lua tests for the same. Fixes Mudlet#8945
Use insertInLine instead of appendLine in TConsole::echo() trigger mode so newlines are embedded in the line text rather than creating new buffer lines. This fixes cecho/creplaceLine regression from #7714 where subsequent echo calls would append to wrong lines. Also adds regression tests for #8945 and #8824.
Brief overview of PR changes/additions
TTextProperties.hso TBuffer and TTextEdit both share the same functions for checking the width of graphemes.setWindowWrapHangingIndent(str:window, int:hangingIndent))appendcallsappendLineandwrapLineand handles buffer shrinking.appendLineadds text to buffer.wrapLinecallsgetWrapInfothen applies wrap, also logs the wrapped lines.getWrapInfochecks the width of each grapheme for determining wrap points instead of the previous method of treating each QChar as width one.More specifically
Created TTextProperties.h for one shared source of grapheme width for both TTextEdit and TBuffer:
graphemeInfo::getBaseCharactercopied over fromTTextEdit::getGraphemeBaseCharactergraphemeInfo::getWidth(unicode, mWideAmbigousWidthGlyphs)copied fromTTextEdit::getGraphemeWidth(unicode)(with some modification)Added
TLuaInterpreter::setWindowWrapHangingIndent/TConsole::setHangingIndentCount/TBuffer::setWrapHangingIndentto allow setting hanging indents for any (mini)consoleAdded
WrapInfo(bool isNewline, bool needsIndent, int firstChar, int lastChar)class to TBuffer.h for storing information on whether to indent / where to snip a line to properly wrap it.Added the inline method
TBuffer::getWrapInfo(const QString& lineText, bool isNewline, const int maxWidth, const int indent, const int hangingIndent)which takes information on the current line of text and returns aQList<WrapInfo>indicating the start / end indices and indentation to wrap the text (empty -> no wrap).I think the code speaks for itself, but hand-wavily the algorithm for wrapping looks like this (glossing over how it handles edge cases and indentation):
Removed
TBuffer::wrap(this is replaced by wrapLine)Changed
TBuffer::wrapLine(int startLine, int screenWidth, int indentSize, TChar& format)toTBuffer::wrapLine(int startLine, int maxWidth, int indentSize, int hangingIndentSize)TBuffer::wrapLinestill loops through each line in the buffer from the startline to the lastline, now calling getWrapInfo for each line and applying the wrap/indentation.const bool isNewline = (time != mudlet::smBlankTimeStamp);.TBuffer::appendLine: simplified this method to just handle appending text to the buffer (by removing handling of shrink buffer / console overflow).Added
appendEmptyLinefor adding a blank line to the buffer and reduce duplicated code.Motivation for adding to Mudlet
Adds functioning hanging indents to all consoles.
Fixes wrapping for 2-width characters (emojis, asian characters, etc.).
Moving all wrapping logic into one method should be much easier to maintain/optimize, as well as hopefully cutting down on the bugs arising from having three different implementations being used in parallel.
Other info (issues closed, discussion etc)
Fixes #3674
Fixes #5564
Fixes #5936
Fixes #6390
Fixes #6432
Fixes #6970
Fixes #7846
Fixes #7892
/claim #5936
/claim #5564
Before benchmark (Mudlet 4.19.1):

After benchmark (commit e6c516e):

Miniconsole + main console showing first line indentation & hanging indentation for single and double width chars:

Script for generating a log to compare old wrapping vs. new