GNU nano - Bugs: bug #51621, softwrapping extends or...
You are not allowed to post comments on this tracker with your current authentication level.
bug #51621: softwrapping extends or foreshortens tabs
Submitter: | Benno Schulenberg <bens> | ||
Submitted: | Sun 30 Jul 2017 07:23:37 AM UTC | ||
Severity: | 3 - Normal | Status: | Fixed |
Assigned to: | dolorous | Open/Closed: | Closed |
Jump to the original submission
Sun 13 Aug 2017 07:09:31 PM UTC, comment #22: |
Benno Schulenberg <bens> |
Sun 13 Aug 2017 04:19:45 PM UTC, comment #21: Looks good to me. I'll update the patch at bug #51671 to account for it. |
David Lawrence Ramsey <dolorous> |
Sun 13 Aug 2017 02:51:24 PM UTC, comment #20: I don't think the prev_index variable is needed. So I suggest the attached version of the patch. |
Benno Schulenberg <bens> |
Sun 13 Aug 2017 09:39:30 AM UTC, comment #19: Okay. I checked on the wrong line, since you didn't mention the number of lines to scroll exactly (which you'vew fixed in the new bug report). Still, it did point me toward problems in the existing patches, and the fixes to them are needed. |
David Lawrence Ramsey <dolorous> |
Sun 13 Aug 2017 08:43:28 AM UTC, comment #18: To comment #14: no, that was not with both patches applied, but with just the first one. As I always say: one issue at a time.
|
Benno Schulenberg <bens> |
Sun 13 Aug 2017 06:28:18 AM UTC, comment #17: ...and that incorrect movement example obviously assumes you have the last patch with proper_x() applied already; it doesn't occur with nano with the new patch applied.
|
David Lawrence Ramsey <dolorous> |
Sun 13 Aug 2017 06:13:49 AM UTC, comment #16: Argh. It turns out that the index shifting is more complex than I thought, and can lead to cases where the cursor doesn't move when it should, which I think is worse than the alternative (without index shifting, vertical positioning is sometimes not quite preserved due to the inability to place an index inside a tab).
|
David Lawrence Ramsey <dolorous> |
Sun 13 Aug 2017 03:34:58 AM UTC, comment #15: Done. Apply the patch in comment #14, then apply the newest patch at bug #51671, and the problem should be gone. |
David Lawrence Ramsey <dolorous> |
Sun 13 Aug 2017 03:29:50 AM UTC, comment #14: I was confused by this, until I checked: the first patch that splits tabs across chunks doesn't cause the problem; the second one on top of it that handles Unicode multi-column spacing does, because it tries to be too clever when melding logic for both cases.
|
David Lawrence Ramsey <dolorous> |
Sat 12 Aug 2017 09:21:51 AM UTC, comment #13: Patch does strange things. To reproduce, run:
|
Benno Schulenberg <bens> |
Sun 06 Aug 2017 03:34:06 PM UTC, comment #12: I figured the patch with less code was better to put first, but okay; attached.
|
David Lawrence Ramsey <dolorous> |
Sun 06 Aug 2017 08:18:02 AM UTC, comment #11: Please provide the patches in reversed order. An ideographic space is a highly unusual case, while tabs are everywhere. So let's first fix the tabs issue. And better attach the ideographic patch to the relevant bug report. |
Benno Schulenberg <bens> |
Sat 05 Aug 2017 11:00:17 PM UTC, comment #10: Argh. In the latest version, there's an off-by-one error in the check for two-column Unicode, and since it's two separate problems, it should be two separate patches (although they conflict if applied separately).
|
David Lawrence Ramsey <dolorous> |
Sat 05 Aug 2017 10:08:14 PM UTC, comment #9: ...and that version was missing the logic to prevent an infinite loop when the terminal is only one column wide.
|
David Lawrence Ramsey <dolorous> |
Sat 05 Aug 2017 09:58:46 PM UTC, comment #8: Sorry, I misread your last post; you wanted the tab check gone, not the lastblank check gone. However, your comment on how tabs were the only multibyte blank character got me thinking.
|
David Lawrence Ramsey <dolorous> |
Sat 05 Aug 2017 08:55:18 PM UTC, comment #7: Okay. Attached a new version that does all of these except the "lastblank_column > goal_column" removal, since that part is actually needed to fix the erroneous behavior noted in comment #4. Specifically:
|
David Lawrence Ramsey <dolorous> |
Sat 05 Aug 2017 12:57:13 PM UTC, comment #6: Thanks for the patch. But... all these calls of move_index_past_partial_tab() happen after a call of actual_x(). Better replace those latter five calls with calls to, say, proper_x(), which first does an actual_x() and then, for the non-tiny version and for the softwrap case, checks whether to increment the x.
|
Benno Schulenberg <bens> |
Fri 04 Aug 2017 08:55:53 PM UTC, comment #5: One last version of the patch is attached.
|
David Lawrence Ramsey <dolorous> |
Fri 04 Aug 2017 04:32:02 AM UTC, comment #4: Here's an improved version of the last patch, again.
|
David Lawrence Ramsey <dolorous> |
Thu 03 Aug 2017 04:45:13 PM UTC, comment #3: Addendum: the display glitch when displaying tabs in whitespace display mode also exists in 2.8.0 (before the variable-width softwrap overhaul) and in 2.7.5 (before the initial softwrap overhaul), so it's more long-standing than I thought. Maybe there should be another flag in display_string() that softwrap mode can use, that indicates "if this line starts with an incomplete tab, don't display the whitespace character for tabs at the start of it"? But that's a separate issue, then. |
David Lawrence Ramsey <dolorous> |
Wed 02 Aug 2017 03:37:14 PM UTC, comment #2: Here's an improved version of the last patch.
|
David Lawrence Ramsey <dolorous> |
Wed 02 Aug 2017 08:21:37 AM UTC, comment #1: Attached a crude attempt at fixing this, against git ac2c81f. It works as follows:
|
David Lawrence Ramsey <dolorous> |
Sun 30 Jul 2017 07:23:37 AM UTC, original submission:
To reproduce, make your terminal sixty columns wide. Then run:
|
Benno Schulenberg <bens> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 20 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2017-08-28 | bens | Open/Closed | Open | Closed | |
2017-08-13 | bens | Status | In Progress | Fixed | |
2017-08-13 | bens | Attached File | - | Added 0001-softwrap-don-t-push-a-straddling-tab-to-the-next-chu.patch, #41504 | |
2017-08-13 | dolorous | Attached File | - | Added 0001-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41500 | |
Attached File | - | Added tabtest4.txt, #41501 | |||
2017-08-13 | dolorous | Attached File | - | Added 0001-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41498 | |
2017-08-06 | dolorous | Attached File | - | Added 0001-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41437 | |
2017-08-05 | dolorous | Attached File | - | Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41432 | |
Attached File | - | Added 0002-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41433 | |||
2017-08-05 | dolorous | Attached File | - | Added 0001-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41431 | |
2017-08-05 | dolorous | Attached File | - | Added 0001-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41430 | |
2017-08-05 | dolorous | Attached File | - | Added 0001-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41424 | |
Attached File | - | Added atblankstest.txt, #41425 | |||
2017-08-05 | bens | Status | None | In Progress | |
Assigned to | None | dolorous | |||
2017-08-04 | dolorous | Attached File | - | Added 0001-softwrap-don-t-break-softwrapped-chunks-before-parti.patch, #41418 | |
2017-08-04 | dolorous | Attached File | - | Added tabfix3.patch, #41407 | |
2017-08-02 | dolorous | Attached File | - | Added tabfix2.patch, #41391 | |
2017-08-02 | dolorous | Attached File | - | Added tabfix.patch, #41390 | |
2017-07-30 | bens | Carbon-Copy | - | Added dolorous |
Powered by Savane 3.13-f8d8.
Corresponding source code
Thanks. Pushed to master, 65d16551.