Re: [Nano-devel] Softwrap navigation overhaul

[Benno Schulenberg]

On Tue, Feb 7, 2017, at 00:06, David Ramsey wrote:
> (Incidentally, I usually have constant cursor position display turned
> on, and when there's only one row in the terminal, it covers all the
> text, so I had to turn it off temporarily to fix this.  Should constant
> cursor position display be automatically disabled on such small
> terminals?)

Ehm...  No.  The user can switch that off himself if he doesn't want it.
(When the terminal gets too narrow, I've made it so that linenumbers
get switched off automatically, but that was because things would crash
when the available space for text became zero.  Plus: what is the use
when all that can be shown is the linenumbers and nothing of the text?
With constant cursor display, the cursor position only overwrites the
text for horizontal movements; the user can stil see the text if he 
does a quick <Up><Down>.)

> > Please remove the optimization that "steps" multiple chunks at a time.
> > Just step one chunk at a time.  Things will become straightforward and
> > won't need any comments at all.
> 
> Done.

Well...

+       for (i = nrows; i > 0; i--) {
+           if (*line == openfile->fileage && current_chunk == 0)
+               break;
+
+           if (*line != openfile->fileage) {
+               size_t prev_last_chunk = (strlenpt((*line)->prev->data) / 
editwincols);
+
+               if (current_chunk == 0 && prev_last_chunk > 0) {
+                   *line = (*line)->prev;
+                   current_chunk = prev_last_chunk;
+                   continue;
+               }
+           }
+
+           if (current_chunk > 0) {
+               current_chunk--;
+               continue;
+           }
+
+           *line = (*line)->prev;
+           current_chunk = 0;
+       }

That is still far from straightforward.  I would write the above as:

       for (i = nrows; i > 0; i--) {
           if (current_chunk > 0) {
               current_chunk--;
               continue;
           }

           if (*line == openfile->fileage)
               break;

           *line = (*line)->prev;
           current_chunk =  (strlenpt((*line)->prev->data) / editwincols);
       }

Also:

+    /* Don't move fewer than one chunk. */
+    if (nrows < 1)
+       return 0;

Is it possible for the chunk routines to get called with nrows = 0?


Testing after applying all 32 patches.  The logic for how to scroll
when inserting a short/long file doesn't work as expected.  Take
the attached file, download it.  Then run:

  stty cols 44 && stty rows 24 && src/nano --soft +185 NEWS

Then type ^R toast <Enter>.  The cursor goes to the bottom of the
screen.  But it has inserted nearly two screenfuls of text, which
means it should center the cursor line -- whenever more than a
screenful of text is inserted, it should do this.  I don't know
where this goes wrong; I'm just testing.  (After an undo, then
a redo /will/ center the cursor line, as it should.)

No further comments today.

Testing was against the latest patch set, 3b-real.

Benno

-- 
http://www.fastmail.com - The professional email service

toast
Description: Binary data