bugGNU nano - Bugs: bug #52125, with softwrap, <Up>...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #52125: with softwrap, <Up> sometimes does not move in the presence of overwide tabs

Submitter:  Benno Schulenberg <bens>
Submitted:  Wed 27 Sep 2017 05:43:51 PM UTC
   
 
Severity:  3 - Normal Status:  Fixed
Assigned to:  bens Open/Closed:  Closed

Jump to the original submission

Thu 12 Oct 2017 06:54:40 PM UTC, comment #10: 

Fixed in git, 7a3a45e6.

Benno Schulenberg <bens>
Group administrator
Fri 29 Sep 2017 02:59:01 AM UTC, comment #9: 

...and those are with varying cursor positions (at the beginning of a tab and several positions past the beginning of a tab).

David Lawrence Ramsey <dolorous>
Group Member
Fri 29 Sep 2017 02:54:48 AM UTC, comment #8: 

More testing: 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.

David Lawrence Ramsey <dolorous>
Group Member
Thu 28 Sep 2017 08:16:14 PM UTC, comment #7: 

After some quick testing with it (from varying positions in the line, using both Up and PageUp, with --smooth added and without, and with tab sizes of 74, 84, and 184), it does seem to work so far.  I'll keep testing it to be sure.

David Lawrence Ramsey <dolorous>
Group Member
Thu 28 Sep 2017 07:52:03 PM UTC, comment #6: 

Attached is a patch that improves the behavior.  But while testing it, I found bug #52139.

Please test whether this "unstuck" patch always prevents <Up> from becoming a no-op.

(file #41950)

Benno Schulenberg <bens>
Group administrator
Thu 28 Sep 2017 05:45:48 AM UTC, comment #5: 

Argh.  It turns out that the formula in proper_x() is probably what needs to be adjusted, as opposed to what my last patch does.  At least the patch exposes the problem a bit more clearly.  Sorry for the confusion.

Specifically, with the cursor not at the start of a tab, <Up> moves the cursor single lines with the patch applied (or not at all with the patch not applied), while <Down> moves it by double lines.

Since the patch makes the behavior of <Up> completely override proper_x(), it means that proper_x() is shifting current_x forward too much in the example case you give: this effectively undoes <Up>, and makes <Down> move too far down.

David Lawrence Ramsey <dolorous>
Group Member
Wed 27 Sep 2017 10:58:23 PM UTC, comment #4: 

Forget the previous comments regarding proper_x().  I've attached a patch that improves the behavior somewhat.  (I deleted the previous one; I attached it by accident, and it didn't work as well anyway; you want the one attached to this comment.)

It's crude, but it seems to work.  It merely catches whether the shifted cursor position is identical to the old one, and, if so, resets leftedge and current_x and recalculates them with actual_x() instead of proper_x(), to ensure that movement occurs.

The only oddity is as follows: moving down isn't quite consistent with moving up.  To see what I mean, use your example, and run:

src/nano --ignore --softwrap -T84 NEWS

Type <Down> 3 times, then type <Right> so that you don't start on the tab.  Then press <Up> twice to go back to the beginning of the tab on line 2.  Then press <Down> once; you'll be where you were before pressing <Up> twice.

(file #41906)

David Lawrence Ramsey <dolorous>
Group Member
Wed 27 Sep 2017 10:07:52 PM UTC, comment #3: 

Although, if I go by the comment:

"If the index lands on a tab, and this tab starts on an earlier chunk, and the tab ends on this row OR we're going forward, then increment the index and recalculate leftedge."

In this particular case, moving up does make the index land on a tab.  However, the tab doesn't start on an earlier chunk, and it doesn't end on this row; it starts on this chunk, and ends on a later row.  The formula should probably be extended to account for this case.

David Lawrence Ramsey <dolorous>
Group Member
Wed 27 Sep 2017 09:54:37 PM UTC, comment #2: 

This is yet another manifestation of the problem where nano is trying to both move to the previous line and keep the cursor at the same vertical position, but can only do one of the two.  The formula you added to proper_x() probably needs to be adjusted for this case; I'd do it myself, but the formula is so complex I can barely understand it, despite the comment (unless you're willing to take me through it?).

(file #41905)

David Lawrence Ramsey <dolorous>
Group Member
Wed 27 Sep 2017 06:37:26 PM UTC, comment #1: 
  • anywhere in the current line of text: <Up> does not work.
Benno Schulenberg <bens>
Group administrator
Wed 27 Sep 2017 05:43:51 PM UTC, original submission:  

To reproduce, run on an 80-column terminal:

  src/nano  --ignore --softwrap  -T84  NEWS

Then type <Down> three times so that the cursor is on the "w" of "wider".  Now type <Up> and then <Down>.  They work, fine.  Now type <Right> and <Left>, then try <Up> again.  The cursor does not move!  Type <Left> and then <Down>.  The cursor can go to the preceding Tab, it's just <Up> that refuses to go there.  Move the cursor anywhere in the current line of text: <Up>

Bug exists since at least nano 2.8.7.

Benno Schulenberg <bens>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #41950:  get-unstuck.patch added by bens (604B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dolorous (Updated the item)
  • -email is unavailable- added by bens (Submitted the item)
  • -email is unavailable- added by bens
  •  

    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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-11-19 bens Open/ClosedOpen Closed
    2017-10-12 bens StatusIn progress Fixed
        Assigned toNone bens
    2017-09-28 bens Attached File- Added get-unstuck.patch, #41950
        StatusNone In progress
    2017-09-27 dolorous Attached File- Added experimental-tab-improvement.patch, #41906
    2017-09-27 dolorous Attached File#41905 Removed
    2017-09-27 dolorous Attached File- Added experimental-tab-improvement.patch, #41905
    2017-09-27 bens Carbon-Copy- Added dolorous

    Back to the top

    Powered by Savane 3.15-e6e5.
    Corresponding source code