patchGNU nano - Patches: patch #9414, weeding: remove redundant...

 
 

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

patch #9414: weeding: remove redundant place_the_cursor() call from spotlight()

Submitter:  David Lawrence Ramsey <dolorous>
Submitted:  Sat 22 Jul 2017 05:57:36 AM UTC
   
 
Priority:  3 - Low Status:  Wont Do
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Release:  None

Mon 15 Jun 2020 07:53:47 AM UTC, comment #3: 

Obsoleted by other changes.

Benno Schulenberg <bens>
Group administrator
Tue 25 Jul 2017 07:44:02 PM UTC, comment #2: 

Yes, edit_refresh() calls place_the_cursor(), but... I think edit_refresh() should not call place_the_cursor() at all.  In most cases that edit_refresh() is called, either 1) the cursor is off, so it is wasteful to place the cursor, or 2) place_the_cursor() is called within ten lines, because it is necessary, or 3) immediately after refreshing the edit window, the prompt bar is redrawn and the cursor is placed there.

The edit_refresh() function seems to have been assigned three tasks: a) determine whether current is still in screen range and scroll when not, b) redraw the entire screen according to the current edittop, and c) place the cursor.  I think these three tasks should be in separate functions, that are only called when needed.  For example, when passing edit_refresh() to do_prompt(), all that edit_refresh() is meant to do is to redraw the edit window exactly as it was (there is no way that edittop could have changed), and there is no need to place the cursor (because it will be placed in the prompt bar).

Further, there is more redundancy in do_replace_loop().  When something needs to be interactively replaced, the first it does is call edit_refresh().  But what it should do is: scroll if needed, but if not, don't bother to refresh, because there is no need.  Then, near the end of the loop, when the user chose to make a replacement (but did not choose "All"), then edit_refresh() is called when there are color strings.  This seems wasteful when the next occurence of the string is way off screen.  So maybe this second edit_refresh() should be replaced with a "refresh_needed = TRUE" (in case this turns out to be the last replacement made), and the first edit_refresh() in the loop can stay?

Benno Schulenberg <bens>
Group administrator
Sat 22 Jul 2017 04:03:45 PM UTC, comment #1: 

Check that.  In all the places that call spotlight(), edit_refresh() is only called just before the first spotlight() call that turns the highlight on, not before the second one that turns the highlight off.  So this should only work for the first call, but in actual practice, it works for both.

This is because the screen doesn't get updated after the second call, except via edit_refresh(), which redraws everything and ignores the highlight.  In turn, this means that having spotlight() turn the highlight off is also unnecessary.

The updated attached patch removes the place_the_cursor() call in spotlight(), as before, but also removes the spotlight() calls that turn the highlight off.  There's also a second patch included, which removes the active parameter from spotlight(); if all the function does is turn the highlight on, active is no longer used.

(file #41289, file #41290)

David Lawrence Ramsey <dolorous>
Group Member
Sat 22 Jul 2017 05:57:36 AM UTC, original submission:  

All the places that call spotlight() call edit_refresh() just before it, and edit_refresh() calls place_the_cursor() itself.  spotlight() thus places the cursor again, in the same spot that edit_refresh() just did; this is unnecessary.

The attached patch against git 4d54231 fixes this.

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
    2020-06-15 bens Priority5 - Normal 3 - Low
        StatusNone Wont Do
        Open/ClosedOpen Closed
    2017-07-22 dolorous Attached File- Added 0001-weeding-remove-redundant-spotlight-and-place_the_cur.patch, #41289
        Attached File- Added 0002-weeding-remove-unused-active-parameter-from-spotligh.patch, #41290
    2017-07-22 dolorous Attached File- Added 0001-weeding-remove-redundant-place_the_cursor-call-from-.patch, #41288

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code