patchGNU nano - Patches: patch #9411, remove cap on moving back chunks...

 
 

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

patch #9411: remove cap on moving back chunks again, and fix bugs caused by the removal

Submitter:  David Lawrence Ramsey <dolorous>
Submitted:  Fri 21 Jul 2017 12:45:20 AM UTC
   
 
Priority:  5 - Normal Status:  Done
Privacy:  Public Assigned to:  dolorous
Open/Closed:  Closed Release:  None

Sun 23 Jul 2017 10:23:42 AM UTC, comment #4: 

Great.  Thanks!  Pushed, 6f9bb53b -- with some changes to the comments and the commit message.

Benno Schulenberg <bens>
Group administrator
Fri 21 Jul 2017 07:03:14 PM UTC, comment #3: 

And, for the record, a hopefully clear explanation of why this works:

If current hasn't gone offscreen, edit_refresh() leaves the viewport alone.  It then redraws all lines, and calls place_the_cursor() to put the cursor where current is relative to edittop.  Any changes to current_y get overridden, but current_y stays in screen range regardless.

If current has gone offscreen, edit_refresh() adjusts the viewport.  If we're not in smooth scrolling mode, it uses CENTERING scrolling mode, which ignores current_y and centers the screen based on edittop.  If we are in smooth scrolling mode, it uses STATIONARY scrolling mode, which uses current_y to put the cursor on the last line of the screen relative to edittop.  It then redraws all lines, and calls place_the_cursor() to put the cursor where current is relative to edittop.  If we're not in smooth scrolling mode, this will be the center of the screen.  If we are in smooth scrolling mode, this will be the last line of the screen.  The changes to current_y are only relevant in smooth scrolling mode (just as they are in do_last_line()), but current_y stays in screen range regardless.

In the latter case, the only way the viewport gets scrolled is if current is offscreen, and if we've uncut or inserted enough lines to move current offscreen and we're using smooth scrolling mode, current should always be the last line when we put it back onscreen.  And by setting current_y to the last row of the screen before refreshing, current is properly set to the last line by the refresh.  This is not only more correct behavior, but it's accomplished with simpler code.

David Lawrence Ramsey <dolorous>
Group Member
Fri 21 Jul 2017 03:42:12 PM UTC, comment #2: 

Updated, even simpler version attached, against git 4d54231.

Calling edit_refresh() manually isn't needed; only avoiding calling place_the_cursor() before updating the viewport is.  The new version leaves everything as it is, except that it removes the cap, and then unconditionally sets current_y to editwinrows - 1, which properly biases viewport updates toward the bottom of the screen if we go offscreen in smooth scrolling mode, just as the unconditional place_the_cursor() call did.  (This is similar to do_last_line().)

(file #41279)

David Lawrence Ramsey <dolorous>
Group Member
Fri 21 Jul 2017 01:14:24 AM UTC, comment #1: 

Updated version attached.  The previous one didn't quite retain the previous behavior when smooth scrolling was on; this one does.  And, for the record, it's against git ee8681d.

(file #41272)

David Lawrence Ramsey <dolorous>
Group Member
Fri 21 Jul 2017 12:45:20 AM UTC, original submission:  

The reason uncutting and inserting don't update the screen properly, as in bug #51514, when the cap on moving back softwrapped chunks is removed is because they set current_y incorrectly, and this is because they do things in the wrong order.

place_the_cursor() assumes that the viewport is up to date, i.e., that edittop is in range of current.  Refreshing the screen, either directly via edit_refresh() or indirectly via refresh_needed = TRUE (which calls edit_refresh() later), updates the viewport and calls place_the_cursor().  Uncutting and inserting need to keep current_y up to date to account for the lines they added, so they call place_the_cursor() first and refresh the screen indirectly second, which is backwards; the cap kept things working properly by forcing current_y into screen range regardless, but it's still a misuse of the code, not to mention redundant (calling place_the_cursor() twice can also be expensive in softwrap mode).

The attached patch removes the cap again, and makes uncutting and inserting use manual edit_refresh() calls instead, which does everything in the proper order.

David Lawrence Ramsey <dolorous>
Group Member

 

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

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

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-28 bens Open/ClosedOpen Closed
    2017-07-23 bens StatusNone Done
        Assigned toNone dolorous
    2017-07-21 dolorous Attached File- Added 0001-tweaks-ensure-that-current_y-stays-in-range-when-unc.patch, #41279
    2017-07-21 dolorous Attached File- Added 0001-tweaks-ensure-that-current_y-stays-in-range-when-unc.patch, #41272
    2017-07-21 dolorous Attached File- Added 0001-tweaks-ensure-that-current_y-stays-in-range-when-unc.patch, #41271

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code