bugGNU nano - Bugs: bug #42189, Searching with softwrap on...

 
 

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

bug #42189: Searching with softwrap on sometimes wedges the cursor in the lower right corner

Submitter:  Benno Schulenberg <bens>
Submitted:  Thu 24 Apr 2014 07:50:17 PM UTC
   
 
Severity:  3 - Normal Status:  Fixed
Assigned to:  bens Open/Closed:  Closed

Jump to the original submission

Mon 31 Oct 2016 09:47:21 AM UTC, comment #11: 

(I don't like the solution much, though.  With the reproduction recipe, 'nano --soft +151 doc/faq.html', and then searching for the $ regular expression, and keeping hitting M-W, every time the screen needs to scroll, first the cursor sits at the start of the statusbar for half a second, or a quarter second, and then the screen is repainted and the cursor sits somewhere in the middle.  The /slowness/ is annoying, and the presence of the cursor for a fraction of a second on the statusbar is annoying too.)

Benno Schulenberg <bens>
Group administrator
Wed 14 Sep 2016 11:13:48 AM UTC, comment #10: 

The fix causes a new bug, though: bug #49088.

Benno Schulenberg <bens>
Group administrator
Wed 14 Sep 2016 09:23:55 AM UTC, comment #9: 

Fixed in git, fc9c7b4.

(The scrolling "a line too much" was a misunderstanding by me: when searching for something, any offscreen occurrence gets centered.  But since that line with "section" is so very long, the cursor lands so low on the screen that I thought the intended scrolling method was FLOWING -- in which case it would have scrolled too much.  But it hasn't; things are fine.)

An easier reproduction recipe is to start 'src/nano --soft +151 doc/fac.html' (without any fiddling with the terminal) and then search for the end-of-line regular expression ($).  When you keep pressing M-W, soon enough you would encounter a case where it doesn't scroll the line fully into view, and where it places the cursor seemingly somewhere in the middle of the line.

Benno Schulenberg <bens>
Group administrator
Tue 13 Sep 2016 03:22:29 PM UTC, comment #8: 

Okay.  Your version is definitely simpler, so if it fixes the problem, go with it.  (Mine started out something like it, but I revamped it trying to fix where the problem would still occur while searching backwards for "section" past the beginning of the file, but your version fixes that too.  And yes, line should have been initialized to edittop->lineno in mine.)  The problem with scrolling an extra line that's in both patches is less of a problem than this one.

David Lawrence Ramsey <dolorous>
Group Member
Tue 13 Sep 2016 08:41:35 AM UTC, comment #7: 

Thanks.  Hmm...  This means that every Ctrl+Left/Right incurs a while loop over a bunch of lines on the screen, and (at first sight) completely unneeded.  Not a big deal, but a bit of a waste.  Attached patch seems to achieve the same effect.

[Also, I would think that in your patch 'line' would need to be initiliazed to edittop->lineno, not to zero?]

The reproduction recipe has changed a bit: the terminal needs to be 31 rows now (and 120 columns) to see the effect when searching for "section" in 'src/nano --soft +1 doc/faq.html'.

Both your and my patch make nano scroll a line more than necessary to get the second "section" onto the screen, but that is a minor issue and I could live with.

(file #38500)

Benno Schulenberg <bens>
Group administrator
Tue 13 Sep 2016 07:11:23 AM UTC, comment #6: 

I've tried to fix this properly with the attached patch against git 4de0a50.  If I understand things correctly, the problem seems to be that the edit_redraw() check for when the current line is offscreen doesn't account for when the current line would be onscreen with softwrap off, but would be offscreen with softwrap on.

(Incidentally, I've tested this patch on bug 47710 as well, but it doesn't fix that bug.)

(file #38499)

David Lawrence Ramsey <dolorous>
Group Member
Sat 21 Jun 2014 11:04:51 AM UTC, comment #5: 

I don't like the behaviour much when the crude patch is applied, keeping the cursor all the time in the centre of the screen -- it changes too much from the normal behaviour, where nano won't scroll as long as there are other search results on the screen.

But... maybe I will apply the patch anyway, because it is an easy and effective solution, and apparently not many people are using softwrap, otherwise we should have had this report quite a while ago.

Benno Schulenberg <bens>
Group administrator
Fri 20 Jun 2014 12:19:38 PM UTC, comment #4: 

In the meantime, to deal with this problem without massive refactoring, there are three edit_redraw() calls in search.c that lead to this problem (which are incidentally fixed by my earlier crude patch that makes edit_redraw() work differently in softwrap mode): one in do_search(), one in do_research(), and one in do_find_bracket().  If all three cases can be adjusted for softwrap mode, that is another way to fix this bug.

David Lawrence Ramsey <dolorous>
Group Member
Mon 16 Jun 2014 08:43:27 PM UTC, comment #3: 

Looking into this some more to try to improve it, it seems that all the movement code (do_up(), do_down(), etc.) has special handling for softwrap mode, except for the movement induced by the search code, which doesn't do the extra scrolling/updating everything else does.

The best fix would probably be to generalize softwrap handling.  Instead of manually adding to current_y and updating in those cases, it would be handled as a virtual y-coordinate which would be adjusted based on line length.

There's already support for virtual x-coordinates using placewewant and xplustabs().  This hypothetical support for virtual y-coordinates would add e.g. linewewant and ypluswraps(), used only in softwrap mode.  However, adding this is nontrivial.

David Lawrence Ramsey <dolorous>
Group Member
Tue 10 Jun 2014 10:50:05 PM UTC, comment #2: 

Addendum: Updated the patch for SVN r4955, adding proper #ifdefs for NANO_TINY in the process.  (Cosmetically, I'm unsure how to add the #ifdefs without breaking the line, so I've broken the line for now.)

(file #31535)

David Lawrence Ramsey <dolorous>
Group Member
Fri 06 Jun 2014 12:16:48 AM UTC, comment #1: 

I've looked into this.  In SVN r4943, after doing your search, when the cursor is wedged, if you hit the Home key, the cursor will return to the beginning of the line.  At this point, holding the right arrow will scroll the cursor forward through the line as expected, but as soon as it reaches the lower right corner of the screen, the screen doesn't scroll, leading to the cursor's being wedged again.

The problem is somewhere in edit_redraw().  It has no special code to handle softwrap mode, but it apparently needs that in order to update the screen properly.  I'm attaching a patch to fix the problem in the crudest way possible (since I'm having trouble understanding the softwrap code): it always assumes the current line is offscreen and updates accordingly when softwrap mode is on.  This means that it probably updates the screen too much as is, but it can probably be used as the basis for a better fix.

(file #31509)

David Lawrence Ramsey <dolorous>
Group Member
Thu 24 Apr 2014 07:50:17 PM UTC, original submission:  

To reproduce: set your terminal to 30 lines 120 columns.  Switch colouring off (M-Y) to avoid the further complication of not even being able to see where one is. Open doc/faq.html and search for the string "section" (without the quotes).  Then hit M-W to search for the next occurrence.  The cursor goes to the lower right corner and no longer seems to respond to <Left> and <Right>.

A <Down> will reveal that nano should have scrolled the screen seven lines before trying to show the next occurrence.

This is both in SVN and in 2.2.2.

Benno Schulenberg <bens>
Group administrator

 

(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 dolorous (Updated the item)
  • -email is unavailable- added by bens (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-10-31 bens Open/ClosedOpen Closed
    2016-09-14 bens StatusNone Fixed
        Assigned toNone bens
    2016-09-13 bens Attached File- Added 0001-softwrap-scroll-when-landing-on-the-bottom-line-of-t.patch, #38500
    2016-09-13 dolorous Attached File- Added nano-4de0a50-softwrap.patch, #38499
    2014-06-10 dolorous Attached File#31509 Removed
    2014-06-10 dolorous Attached File- Added nano4955softwrap.patch, #31535
    2014-06-06 dolorous Attached File- Added nano4943softwrap.patch, #31509

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code