Thu 12 Oct 2017 07:19:10 PM UTC, comment #12:
Fixed in git, 7a3a45e6. Thanks for the extensive testing.
|
Wed 11 Oct 2017 02:12:10 AM UTC, comment #11:
I've put it through the same testing as before: in an 80x24 terminal, vertical upward movement (both <Up> and <PageUp>, with and without --smooth) works properly using tab sizes of 1, 2, 7, 9, 81, 132, 161, 219, 329, 2001, and 2397.
|
Sat 07 Oct 2017 12:28:59 AM UTC, comment #10:
After some testing, it seems to work (aside from the issue you mentioned, which I can see in the 219-column tab case in my testing of the earlier version). Thanks.
|
Thu 05 Oct 2017 08:08:44 PM UTC, comment #9:
The problem is that the start of a tab can be anywhere. The end of a tab can be computed with a simple formula, but a tab can start anywhere in maximum width that it occupies. So the actual column position of the tab needs to be computed in order to compare it to the left edge. See attached, improved patch.
(This is still not a complete fix for the tabs problem, because the cursor will be misplaced when it is in the middle of a row and moving up and a tab occupies several rows and begins near the end of a row: the cursor will be placed on the tab when it should be placed in the centre of that row. But that will be reported in a different bug.)
Please test the current patch against the current bug.
(file #42071)
|
Sun 01 Oct 2017 10:11:00 PM UTC, comment #8:
I've looked into it more, and proper_x() seems to be what's throwing off the leftedge and target into what you wouldn't expect (as far as I can tell; if you can give me a more specific example of what you don't expect, it would be more useful).
And having the chunk iterators skip over empty chunks wouldn't fix the problem entirely, since the case in comment #3 makes the tab only take up roughly half of one chunk on one row, and the other half on the next row, so neither chunk is empty. Hmmm.
|
Sun 01 Oct 2017 08:33:03 PM UTC, comment #7:
Okay.
Just to be clear, by "a chunk that is completely empty," you mean a chunk where a tab begins before it and ends after it, so that the cursor cannot be placed anywhere on it? ("No character on that row" to me means a blank line, but those can obviously exist.)
go_back_chunks() and go_forward_chunks() probably should move leftedge past such a chunk, but remember that the special handling of tabs was only added in 2.8.6, so they haven't been adapted for that possibility yet, and still assume that one chunk is always one full row at most; the row count would need adjustment when that isn't the case. Not to mention, get_softwrap_breakpoint() would probably need some way to indicate such a chunk.
|
Sun 01 Oct 2017 07:41:12 PM UTC, comment #6:
Hm. I guess the forward part needs a second condition, just like the !forward part has two conditions. Will look into that later.
But... maybe the problem is elsewhere, because when applying the attached diagnostic patch, one sees that going up and landing in the middle of a tab, the leftedge and target are not what one would expect. I would have thought that go_back_chunks() and go_forward_chunks() would never put leftedge on a chunk that is completely empty (there is no character at all on that row). If go_back() and go_forward() wouldn't do that, we wouldn't need these position corrections.
(file #42045)
|
Sun 01 Oct 2017 04:52:44 PM UTC, comment #5:
More info regarding the report in comment #3:
The problem occurs when pressing <Down> on anywhere from column 133 through column 159: that entire row, not counting the columns taken up by the tab.
More info regarding the report in comment #4:
The problem occurs when pressing <Down> on anywhere from column 220 through column 239: that entire row, not counting the columns taken up by the tab.
|
Sun 01 Oct 2017 04:35:00 PM UTC, comment #4:
And with the attached file nanocrap2.txt, run:
./nano --ignore --softwrap -T 219 +1,222 ./nanocrap2.txt
The same problem occurs: <Down> moves two rows, <Up> afterward moves one row.
(Both this test and the test in comment #3 should be run in an 80x24 terminal, by the way.)
(file #42042)
|
Sun 01 Oct 2017 04:28:55 PM UTC, comment #3:
Running the same tests listed in bug #52125 with my nano binary, I've found one (admittedly obscure) case where this problem still happens, even with your last patch applied. With current git and your last patch applied, run:
./nano --ignore --softwrap -T 132 +1,135 nanocrap.txt
Press <Down> once, and the cursor will move down two rows. Press <Up once>, and the cursor will move up only one row; you'll have to press <Up> twice to get back to the row you started on.
The nanocrap.txt file is attached; it contains a small enough chunk of my nano binary to show the problem.
(file #42039)
|
Sun 01 Oct 2017 03:00:29 AM UTC, comment #2:
Okay, that makes sense; thanks for the explanation. And this patch does seem to work after some quick testing, although I still need to do the tests I did with the last patch to make sure.
The tests you added/changed that use (*leftedge - 1) and (editwincols - 1) do make me a bit nervous, given how nano is supposed to still function in a one-column terminal, but they don't seem to cause any problems so far under such circumstances. (Although checking that did lead me to discover bug #52149.)
|
Sat 30 Sep 2017 07:40:25 PM UTC, comment #1:
Okay, I think I've got it solved now. See attached patch.
Trying to explain the formula in words: when we've landed on a tab and we haved moved down (that is: forward) and the end of the row that preceeds the current row is effectively on the same tab as the current landing point, then not moving the index forward would mean we would recede a row from the current one, which is a no-no, so the index must be incremented.
Similarly, when moving up (that is: backward) [and we've landed on a tab] and the end of the preceding row is on the same tab as the landing point AND the end of the current row is NOT on the same tab as the landing point (which means that there is at least a tab character on the current row, maybe even text, but there IS something that we can put the cursor on), then the index must incremented because otherwise the cursor would skip a row that has something tangible on it.
I've dropped the other two conditions from the formula, as I don't remember what they're for (probably just approximations of the right formula).
(file #42019)
|
Thu 28 Sep 2017 07:49:28 PM UTC, original submission:
To reproduce, run on an 80-column terminal:
src/nano --ignore --softwrap -T84 +1,10 NEWS
The cursor will be on the "7" of "27". Now press <Down> twice. The cursor will land on the "i" of "wider". But it now takes three presses of <Up> to get the cursor back to the starting position, on the "7". This is wrong. Any amount of <Up>s or <Down>s should require the exact same amount of the opposite key to get back to the starting point (as long as EOF and BOF are not reached).
It is the second <Down> that takes too large a stride -- it should stop on the tab character on the third row, before going to the "i" on the next row on the third <Down>.
|