GNU nano - Bugs: bug #50621, in current git, an overlong line...
You are not allowed to post comments on this tracker with your current authentication level.
bug #50621: in current git, an overlong line can cause a Badness warning
Submitter: | Benno Schulenberg <bens> | ||
Submitted: | Thu 23 Mar 2017 08:38:08 PM UTC | ||
Severity: | 3 - Normal | Status: | Fixed |
Assigned to: | dolorous | Open/Closed: | Closed |
Jump to the original submission
Tue 28 Mar 2017 06:23:07 PM UTC, comment #28: |
David Lawrence Ramsey <dolorous> |
Tue 28 Mar 2017 05:49:10 PM UTC, comment #27: Ah. Okay. Thanks. Applied to git, 93c62489.
|
Benno Schulenberg <bens> |
Tue 28 Mar 2017 01:50:16 PM UTC, comment #26: Here's a fix. In partial-fix-4a, I forgot to handle the case where the line at the top of the screen was edittop; the compensation isn't needed in that case because of firstcolumn. |
David Lawrence Ramsey <dolorous> |
Tue 28 Mar 2017 12:57:26 PM UTC, comment #25: And scrolling either up or down means either with the arrow keys, or Meta-=/Meta--. |
David Lawrence Ramsey <dolorous> |
Tue 28 Mar 2017 12:25:22 PM UTC, comment #24: No problem. But this isn't quite fixed yet. In an 80x24 screen, with current git, and with your original lotsofexes file, run:
|
David Lawrence Ramsey <dolorous> |
Tue 28 Mar 2017 10:42:03 AM UTC, comment #23: Fixed in git, 66ef8f45. Thanks for the patches. |
Benno Schulenberg <bens> |
Mon 27 Mar 2017 11:06:54 PM UTC, comment #22: Oops, make that partial fix 4a (relative to 4, there's a better comment, and softwrap-specific calculations are only done in softwrap mode). |
David Lawrence Ramsey <dolorous> |
Mon 27 Mar 2017 10:48:47 PM UTC, comment #21: And here's partial fix 4, which seems to fix the other display issues (except for some display issues with the mark, but that's a separate bug). |
David Lawrence Ramsey <dolorous> |
Mon 27 Mar 2017 08:50:46 PM UTC, comment #20: I'm looking into it.
|
David Lawrence Ramsey <dolorous> |
Mon 27 Mar 2017 06:39:09 PM UTC, comment #19: (For cross reference: the Badness that was reported in comment #9 has been rereported as bug #50657, and has been fixed in git -- with a mistake in the commit message: the scrolling directions are reversed.) |
Benno Schulenberg <bens> |
Mon 27 Mar 2017 04:00:00 PM UTC, comment #18: Please have a look at current git and at bug #50653. I think it might fix the redraw problems you are seeing with your time-and-magix text (when combined with your partial-3 fix).
|
Benno Schulenberg <bens> |
Mon 27 Mar 2017 12:59:17 AM UTC, comment #17: Addendum: The change is PageDown behavior is a change from nano's previous behavior, but it seems to be exactly what Pico does (as far as I can tell; since Pico obviously lacks smooth scrolling, I have to add lines until Pico's screen centering behavior lines things up the right way). But this is off-topic. |
David Lawrence Ramsey <dolorous> |
Sun 26 Mar 2017 11:42:02 PM UTC, comment #16: It turns out that the top line not changing in paragraph.txt is a bug in the PageUp/PageDown functions regarding the top line's being scrolled off the screen. I'll report it separately, and I have a fix for it.
|
David Lawrence Ramsey <dolorous> |
Sun 26 Mar 2017 08:52:07 PM UTC, comment #15: If it stops at a nonzero value, it won't update any softwrapped line. I figured that the nrows count should be decremented by the number of rows update_line() returns, not by one for each row (a holdover from before softwrap mode). Here's a patch which adds that.
|
David Lawrence Ramsey <dolorous> |
Sun 26 Mar 2017 08:28:07 PM UTC, comment #14: The for loop needs to stop as soon as update_line() returns a nonzero value. |
Benno Schulenberg <bens> |
Sun 26 Mar 2017 08:02:53 PM UTC, comment #13: Pushing the starting row one step back when scrolling forward should be covered by the nrows increase; if nrows is increased, then the go_forward_chunks() call moves one less row forward. As for backward, that's a good point. The approach here should probably change to accommodate both possibilities. (I think adding 2 to nrows seemed to cover both possibilities before, which is why it was in there before.)
|
David Lawrence Ramsey <dolorous> |
Sun 26 Mar 2017 06:22:01 PM UTC, comment #12: Without your patch applied and with your recipe of 'nano +5,161 --ignore --softwrap paragraph.txt', typing ten times <Down> and then ten times <Up>, I get three warnings:
|
Benno Schulenberg <bens> |
Sun 26 Mar 2017 06:13:54 PM UTC, comment #11: And even with the patch applied, there are still warnings with your lotsofexes file in the first post (they might be caused by the same issues as with the particular file mentioned in the previous post; I don't know yet). Open an 80x24 terminal, and run
|
David Lawrence Ramsey <dolorous> |
Sun 26 Mar 2017 03:31:03 PM UTC, comment #10: Thanks. I've melded your work with mine into the attached patch.
|
David Lawrence Ramsey <dolorous> |
Sun 26 Mar 2017 11:42:00 AM UTC, comment #9: The only Badness warning that remains is when doing:
|
Benno Schulenberg <bens> |
Sun 26 Mar 2017 11:24:39 AM UTC, comment #8: It was unclear how exactly scrolling down was broken by your partial fix... until I realized it was scrolling up (in my parlance) that was broken.
|
Benno Schulenberg <bens> |
Sat 25 Mar 2017 06:06:30 AM UTC, comment #7: And the partial fix breaks non-softwrap mode when scrolling down with the mark off. Stumped again. |
David Lawrence Ramsey <dolorous> |
Sat 25 Mar 2017 06:02:37 AM UTC, comment #6: Here's a patch containing a partial fix, which should also make that code chunk a bit clearer. With the patch applied, nano now only redraws the row before the scrolled region if the mark is on or if it's not on the first "page", as intended.
|
David Lawrence Ramsey <dolorous> |
Sat 25 Mar 2017 03:08:34 AM UTC, comment #5: And the previous comment's references to the mark also apply to horizontally scrolled lines. If you're not on the first "page" and scroll three rows up, you need to redraw the fourth row at the top to account for that too, since current has changed. Otherwise, the line you started on will still be drawn as horizontally scrolled even though it no longer is. |
David Lawrence Ramsey <dolorous> |
Sat 25 Mar 2017 12:22:27 AM UTC, comment #4: Well... I originally wrote edit_scroll() ages ago, I didn't make the comment specific enough, and I can't remember offhand why it's necessary, hence my grasping at straws in an attempt to fix it.
|
David Lawrence Ramsey <dolorous> |
Fri 24 Mar 2017 08:56:33 PM UTC, comment #3: [I don't understand that comment that you're changing with your last patch. How can the rows before /and/ after the scrolled region be both on the screen? Say we scrolled three lines backwards, then all the rows in the edit window have been moved three rows down, and the three rows at the top need to be redrawn with new content. (And the fourth row from the top needs to be redrawn... why exactly?) Sure, there is a line /after/ the scrolled region that is visible on screen ((when scrolling backward), but never any line before it. Mutatis mutandis for scrolling forward. So... why does it add 2 when nrows is not 1?] |
Benno Schulenberg <bens> |
Thu 23 Mar 2017 09:23:35 PM UTC, comment #2: Never mind the last patch: the code is needed not just for horizontally scrolled lines, but also marked ones, so it breaks display of marked text.
|
David Lawrence Ramsey <dolorous> |
Thu 23 Mar 2017 09:00:46 PM UTC, comment #1: I think I know what's causing this: some code in edit_scroll() that's being run unconditionally when it should only be run in non-softwrap mode (since it's supposed to cover horizontally scrolled lines, which don't apply in softwrap mode). The attached patch against current git seems to fix it. |
David Lawrence Ramsey <dolorous> |
Thu 23 Mar 2017 08:38:08 PM UTC, original submission:
To reproduce, first do:
|
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 15 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2017-03-28 | dolorous | Attached File | - | Added 0001-display-don-t-draw-more-edittop-chunks-than-the-scre.patch, #40181 | |
2017-03-28 | bens | Status | In Progress | Fixed | |
Assigned to | None | dolorous | |||
Open/Closed | Open | Closed | |||
2017-03-27 | dolorous | Attached File | - | Added partial-fix-4a.patch, #40176 | |
2017-03-27 | dolorous | Attached File | - | Added partial-fix-4.patch, #40175 | |
2017-03-27 | dolorous | Attached File | - | Added paging-fix.patch, #40173 | |
2017-03-27 | bens | Summary | in current git, an extremely long line can cause a Badness warning | in current git, an overlong line can cause a Badness warning | |
2017-03-26 | dolorous | Attached File | - | Added partial-fix-3.patch, #40153 | |
2017-03-26 | bens | Status | None | In Progress | |
2017-03-26 | dolorous | Attached File | - | Added partial-fix-2.patch, #40148 | |
Attached File | - | Added paragraph.txt, #40149 | |||
2017-03-25 | dolorous | Attached File | - | Added partial-badness-fix.patch, #40124 | |
2017-03-23 | dolorous | Attached File | - | Added 0001-tweaks-improve-comment-in-edit_scroll.patch, #40103 | |
2017-03-23 | dolorous | Attached File | - | Added 0001-display-don-t-handle-horizontally-scrolled-lines-in-.patch, #40102 |
Powered by Savane 3.13-4448.
Corresponding source code
No problem. And yes, that's the right fix; good catch.