Re: having a scroll bar also in softwrap mode

[Marco Diego Aurélio Mesquita]

On Thu, May 7, 2020 at 3:48 PM Benno Schulenberg <address@hidden> wrote:
>
>
> Op 06-05-2020 om 21:32 schreef Marco Diego Aurélio Mesquita:
> > This probably happens because chunk_nr is not renumbered when softwrap
> > is toggled. My solution is to renumber all buffers from the top
> > whenever the screen is resized or when softwrap is toggled.
>
> After some quick tests, the behavior looks good now.  Thanks.
>

You're welcome.

> But renumbering *all* buffers when softwrap is toggled or the screen
> is resized seems like the wrong approach.  I think this should be done
> in redecorate_after_switch() -- there is already a relevant check for
> this there.

Are you suggesting to renumber the buffer in redecorate_after_swtich
on the codition if (ISSET(SOFTWRAP)) ? If that is the case, then this
means there will be a renumbering every time buffers are switched,
right? I think that resizing the window or toggling softwrap are much
less common actions then switching buffers, do you really think this
is a better approach?

Did it anyway.

>  And is it necessary to renumber the buffer also when
> softwrap was turned off?
>

Since totalrows and first_row need chunk_nr, yes it is needed. I think
there is an alternative: when SOFTWRAP is toggled, renumber only when
it is turned on and add a check in draw_scrollbar:

  if (!ISSET(SCROLLBAR)) {
    totalrows = openfile->filebot->lineno;
    first_row = openfile->edittop->lineno;
  }

I have no preference about either.

Did it anyway.

> > +     newnode->chunk_nr = (prevnode) ?
> > +             number_of_chunks_in(prevnode) + prevnode->chunk_nr + 1
> > +             : 1;
>
> This is poor style: three characters on a line by itself?  There is
> more than enough remaining room on the previous line.
>

Fixed on the updated patch.

> >       dst->lineno = src->lineno;
> > +     dst->chunk_nr = src->chunk_nr;
> >  #ifndef NANO_TINY
>
> The chunk_nr should not be present in the tiny version.  Move or
> put the relevant things between #ifndefs.
>

I didn't test compilation with NANO_TINY defined but moved the
chunk_nr references I found inside #ifndef's (or added them).

> > +#ifndef NANO_TINY
> > +     if (thebar)
> > +             draw_scrollbar();
> > +#endif
>
> Why is this needed?  I removed it from your original patch (the one
> without the softwrapping), why has it returned?
>

I think this can be removed. I re-added this because the scrollbar
paddle was overwritten when chars were added to a softwrapped line
which overlapped it. I don't know if my test was flaky or something
else fixed it since I could not reproduce this problem again.

I'm happy to see that we are finally getting close to something
acceptable. Please review it.

0001-Initial-softwrap-support-for-scrollbar.patch
Description: Text Data