Thu 06 Apr 2017 05:55:33 PM UTC, comment #13:
Fixed in git, f3e2ad94.
(Yes, the "Shifted" was just for testing purposes. But it's good to add such things now and then, because it shows for example that somehow reset_cursor() is called unnecessarily when typing ^W, for example.)
|
Thu 06 Apr 2017 12:16:12 PM UTC, comment #12:
Okay; I wasn't certain about the mbwidth() stuff. Looks good to me; thanks. (Does "Shifted" need to be displayed every time the code is run in the final version, or is that just for testing purposes?)
|
Thu 06 Apr 2017 08:34:04 AM UTC, comment #11:
Calling mbwidth() doesn't need checking for UTF-8 support first; that is handled in mbwidth() itself. (The other calls of mbwidth() don't need to be guarded either; I've meaning to remove those, but didn't yet get around to it.)
Also, the "col == editwincols -1" test doesn't work for tabs. So the right and sufficient test is quite simple: when placewewant and col don't agree, shift to next row. See attached patch.
(file #40281)
|
Wed 05 Apr 2017 09:12:54 PM UTC, comment #10:
Addendum to last comment: after some testing, it doesn't seem to cause a problem with End, actually, but since End uses similar logic to Home, I might just be missing something (or being overly paranoid). Either way, it can go in the separate bug report.
|
Wed 05 Apr 2017 08:57:15 PM UTC, comment #9:
The patch seems good as a short-term fix. I did tweak it a bit, though: calling mbwidth() requires checking for UTF-8 support first, as is done elsewhere; and it's not enough to just check if the character is less than 0x20 to see if it's a control character, since it might be a high-bit control character. The tweaked version is attached.
The thing in comment #3 actually applies to both Home and End; I'll report it.
(file #40273)
|
Wed 05 Apr 2017 07:38:25 PM UTC, comment #8:
Attached patch avoids the scrolling problem. It makes cursor behave a bit strange with double-width characters, but... I like it better in fact than the cursor jumping from the left to the right of the screen and back again when we're just holding <Up> or <Down>. What do you think?
(The thing in comment #3 is a separate bug and needs rereporting.)
(file #40272)
|
Wed 05 Apr 2017 07:15:46 PM UTC, comment #7:
Since actual_x() is what causes the issue here, the problem extends to up, down, Home, and End placing the cursor on the wrong line when converting from columns to indexes. Using current_is_above_screen() or current_is_below_screen() won't fix it for up and down, since the line still won't be what the code is expecting with the current softwrap formulas.
Either there has to be some way to adjust the cursor in these particular cases (either a modified actual_x(), or some manual adjustment after the original actual_x()), or chunks have to be changed to variable-length to avoid putting two-column characters (either control or Unicode) on the screen edge in the first place.
I haven't figured out how to do the former without breaking something else, and since 2.8.1 seems to be a relatively quick bugfix release, I figure the latter is out for that release.
|
Mon 03 Apr 2017 11:58:56 PM UTC, comment #6:
There's an easier way to reproduce it, from both the upward and downward directions, without the uncertainty of binary files. This is because two-column Unicode characters, like control characters, can't be split on screen edges; whatever fixes this for one should fix it for the other.
1. Load the attached file chinese-page.html (based on one of the Chinese files in another bug, but with enough text duplicated to make more than one page) with the following options:
stty cols 80 && stty rows 24 && src/nano --ignore --softwrap --constant chinese-page.html
2. Press Down 19 times; nano won't scroll down (visually).
3. Press Ctrl-L to update the screen properly.
4. Press Up 9 times; nano won't scroll up (visually).
(file #40254)
|
Mon 03 Apr 2017 08:14:06 PM UTC, comment #5:
Okay. I can reproduce it when scrolling up (not with your exact example, probably due to differences in binaries), but I'm not surprised that it occurs then too. It relies on the same assumption as going down, but in reverse: that we're on the upper edge of the screen, but shifting backwards due to the control character on the edge puts it on the wrong line.
|
Mon 03 Apr 2017 07:32:34 PM UTC, comment #4:
Indeed, it can occur also when scrolling up (or rather: trying to scroll up). Run:
stty cols 80 && stty rows 24 && src/nano +,1521 --ignore --softwrap --constant src/nano
Then type nine times <Up>. On the ninth time, the cursor fails to move; and on the tenth and subsequent times, the screen fails to scroll.
|
Sat 01 Apr 2017 07:22:31 PM UTC, comment #3:
Also, dynamic home doesn't work quite right if a control character straddles a line; if the second half of the control character is at the beginning of a line, actual_x() shifts it back to the first half at the end of the previous line, which leads to its misdetecting the location of the beginning of the line and moving the cursor back.
|
Fri 31 Mar 2017 05:47:20 PM UTC, comment #2:
Check that; it's not edit_scroll() but near there. It's that the check for being on the last line of the edit window relies on current_y's being (editwinrows - 1), but with the control character's shifting it up in this case, current_y is on the line before that. (do_up() probably has the same problem for the checks for the first line of the edit window, so it should probably get an equivalent fix after this.)
The variable-length chunk changes would fix it by making sure we never straddle a control character on the edge of the screen like that. Alternatively, using current_is_below_screen() to detect whether current has gone offscreen instead of the cheaper way would fix it. Either way, that adds more processing time. Unless there's another (cheaper) way to check if current has gone offscreen when it may not technically be on the last row?
|
Fri 31 Mar 2017 05:38:14 PM UTC, comment #1:
After some testing, it's not getting stuck (as in, not moving from a particular position); it's actually moving offscreen but not doing screen updates when it should.
Try it with the options "--ignore --soft --constantshow" instead of just "--ignore --soft", and you'll see: when it appears to get stuck it really just isn't updating the screen when scrolling down, since the cursor position does change, and pressing Ctrl-L does update it.
Maybe bug #50691 is just another manifestation of the same problem, or maybe not, but it looks as though the problem is in edit_scroll() either way.
|
Fri 31 Mar 2017 09:54:05 AM UTC, original submission:
To reproduce, run:
stty cols 80 && stty rows 24 && src/nano --ignore --soft src/nano
(Yes, that means opening nano's binary with nano itself.)
Now press and hold <Down>. Just before reaching the last row, the cursor gets stuck on the "^" of a "^@" that straddles the row boundary. (Or if your binary is much different, it might get stuck on a replacement character, �, indicating an invalid starter byte.)
|