[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Nano-devel] softwrap navigation overhaul, now breaking on whitespace

From: David Ramsey
Subject: [Nano-devel] softwrap navigation overhaul, now breaking on whitespace
Date: Wed, 1 Mar 2017 15:57:34 -0600

The softwrap navigation overhaul now breaks words on whitespace
boundaries near the edge of the screen.  This fulfills the feature
request in bug 49959, and fixes the problems with two-column characters
in the last version.  This is version 4, consisting of 39 patches.  Note
that the commit logs still need to be put in the proper style, and they
will be soon; I haven't yet because I'm not sure whether tagging most of
the patches with "softwrap:" is clear enough, and I've been more focused
on getting the new code working.

The first 33 patches are mostly as in the last version, barring a few
tweaks (plus a fix for a significant cut-and-paste error; when counting
lines in do_gotolinecolumn() and changing it back, I had it using
editwinrows where it should have been using editwincols): the line
numbering mode refresh fix is gone, having been merged separately, and
in its place, there's an attempt to document dynamic home and dynamic
end on the help screen.

The remaining 6 patches on top of it add softwrapping on whitespace: the
last column of the edit window is now reserved for blanks, and will
never contain text.  This condition is so that blanks that we break
lines on, and the newlines that end the lines, are always accessible;
without this case, if a blank or newline is exactly on the screen edge,
it will be inaccessible and break the display.  Also, two-column
characters will never be split between rows.  This mostly applies to
Unicode, which can't be split between rows anyway, but also applies to
control characters, which technically could be split this way but no
longer will be; I think this change is okay, though.

(I've tried to make this as robust as I can, but there are two issues
with two-column Unicode characters under extremely narrow terminal sizes
that I have no idea how to fix.  nano will malfunction if it gets a
two-column Unicode character when the terminal is only one column wide,
as you'd expect.  It will also malfunction if it gets a two-column
character when the terminal is two columns wide, since the last column
is reserved for blanks including null terminators.

Specifically, displaying a two-column character at the start of a line
when the terminal only has two columns tries to break the character at
(editwincols - 1), which is impossible because it's 1 and in the middle
of the character; the only other places it could break are 0, which is
where the line starts, so the search for the breakpoint gets stuck at
the start of the line and becomes an infinite loop; and 2, which is
after the character and on the next line, so a blank immediately after
the character becomes inaccessible and the display breaks.  If the
terminal is at least three columns wide, neither of these problems
manifest, and everything works as expected.)

Finally, placewewant is calculated relative to the current leftedge in
softwrap mode now.  With chunk lines' now being of arbitrary length
instead of constant, there was no longer any easy way to translate
placewewant between non-softwrap and softwrap mode.

As for how it's implemented, I couldn't use any code from Mark Majeres'
old patch.  The code is too different now, and the assumptions have
changed enough that his way of caching information on softwrapped lines
won't work either.  Also, his use of break_line() to get where to break
the line is overkill, since softwrap works much more with columns than
indexes, and the assumptions on breaking lines are different
(break_line() goes past the breakpoint in some cases, while softwrapped
lines have to stay within the screen and can never go past the
breakpoint), so my code uses a function based on break_line() instead.
My code also uses a boolean and size_t's instead of ssize_t's set to -1,
since the other softwrap code uses size_t's exclusively, and mixing the
two might lead to problems (although I might just be paranoid).

Specifically, Mark's patch's tying cached information to screen updates
relies on shifting update_line() calls, and some of those calls don't
exist anymore in the current codebase.  Also, it fails to work as soon
as you try to move to an offscreen line that hasn't been updated yet.
Furthermore, even if it did work, it's not enough as is, since it
assumes that we can't navigate within the line and only stores the
number of softwrapped rows each line takes up.  On top of what I've
done, it would have to cache not just a row count, but the columns and
possibly indexes of the leftedges of each softwrapped line.

Additionally, there's the issue of what to do if the user switches
softwrap mode off, changes some text, and switches it back on: there
would have to be some boolean to indicate "this line has changed,
recalculate its softwrapped chunk info and don't use the cached values".
Also, his code doesn't account for moving past the breakpoint of a
softwrapped line via direct column movement (i.e., clicking somewhere
with do_mouse(), or moving somewhere with placewewant): since we're not
breaking the line but only appearing to, actual_x() will erroneously
shift us forward to the next chunk by that many characters.

In terms of caching to improve performance, the only way I can think of
to do it properly is to cache the information associated with a file
when it's first read in, use the cached information whenever possible,
and then update the cached information on any given line when the text
changes.  This will require putting hooks everywhere the text changes,
which is a more massive undertaking than the softwrap overhaul, and
while it may be necessary, is outside the scope of it: nano uses enough
common code that some parts are easier to do this for than others (for
example, putting hooks in ingraft_buffer() and extract_buffer() will
handle everything that passes through them).

Thus, the new softwrap code still calculates everything on the fly, just
as it did before, even if the formulas are different now.  This does
make things somewhat slower by definition, but it works as far as I can

It's against git 83a841c.  Attached.

Description: Zip archive

reply via email to

[Prev in Thread] Current Thread [Next in Thread]