Re: having a scroll bar also in softwrap mode

[Marco Diego Aurélio Mesquita]

On Mon, Apr 27, 2020 at 7:35 AM Benno Schulenberg <address@hidden> wrote:
>
>
> [Please use a fitting subject line when changing the subject, so things
> stay findable in the archive, and best start a new thread.]
>
> Op 26-04-2020 om 20:54 schreef Marco Diego Aurélio Mesquita:
> > Attached patch makes scrollbar behave correctly in softwrap mode. It
> > is a bit of a waste to redraw the scrollbar every time a softwrapped
> > line is drawn or modified but it is very simple.
>
> The patch partially works, but breaks line addressing.  When I do:
>
>   src/nano --ignore --soft --line +57 doc/faq.html
>
> the cursor lands on line 68 (it depends on the size of the terminal).
>

The problem with line addressing is probably because I changed line
numbering to logical lines to make debugging easier. If this is the
cause, then that is easy to fix.


> Make your terminal 35 rows and 100 columns, then run:
>
>   src/nano --ignore --soft --line  doc/faq.html
>
> And type: ^V ^V x
>
> See that when you type x, the scrollbar jumps one character up, even
> though the number of chunks was not changed.  Now type <Bsp> to remove
> the x.  See that scrollbar does not move.  Something is not right.
>

This is probably caused because the renumbering is only triggered when
there is a change on a softwrapped line. What really is needed is a
way to detect if the number of softwrapped lines has (chunks) has
changed. The code would somewhat remember his:

    int number_of_chunks_before = number_of_chunks_in(changed_line);
    [apply changes to line]
    if(number_of_chunks_in(changed_line) != number_of_chunks_before)
      renumber_from(changed_line);

This would have to be called every time and change is made. That is
the reason why I put it on the undo code. Can you point me to
somewhere in the code where this could be done?

>
> Oh, also: the patch does not fully compile:
>   winio.c:2752:3: warning: implicit declaration of function ‘draw_scrollbar’;
>

Easy to fix.

>
> >       ssize_t lineno;
> >               /* The number of this line. */
> > +     ssize_t llineno;
> > +             /* The number of this logical line. */
>
> Poor naming of a variable: I hardly see a difference with lineno.
>

How about logical_lineno?

> > @@ -941,6 +941,11 @@ void add_undo(undo_type action, const char *message)
> >       undostruct *u = nmalloc(sizeof(undostruct));
> >       linestruct *thisline = openfile->current;
> >
> > +     /* If a softwrapped line is modfied, we may need to renumber
> > +      * logical lines */
> > +     if(ISSET(SOFTWRAP) && number_of_chunks_in(openfile->current))
> > +             renumber_from(thisline);
>
> This is not the right place for this.  Even when all undo code is absent,
> things should work.  Furthermore, you have this code also in update_undo(),
> which means that for most operations the renumbering will be done *twice*.
> That is not acceptable.
>

Is there a point of the source code that is called whenever a change
to a line is made?

> > +     /* If drawing a softwrapped line, we may need to renumber logical 
> > lines */
> > +     if(ISSET(SOFTWRAP) && number_of_chunks_in(openfile->current))
> > +             draw_scrollbar();
>
> Comment does not fit the code.  :/
>

Easy to fix.

> What if number_of_chunks() changed from 1 to 0 by the last operation?
> Also then the scroll bar needs to be redrawn.
>

Same as a point before this. A way to fix this is to detect changes in
the number of chunks (logical lines) every time a line is changed.

Thanks!