Re: how should dragging work, and is it worth it?

[Benno Schulenberg]

Op 19-06-2020 om 10:21 schreef Marco Diego Aurélio Mesquita:
> Now, what is your opinion on my approach?
> 
> 
> 0001-possible-new-feature-allow-the-scrollbar-paddle-to-b.patch
> 
>  #ifndef NANO_TINY
> -int *bardata = NULL;
> -             /* An array of characters that together depict the scrollbar. */

Why do you remove the bardata?
And is that a necessary change to get the dragging to work?

> +             ssize_t row_count = (openfile->current_y >= 0 && 
> openfile->current_y < editwinrows) ?
> +                             click_row -openfile->current_y  :
> +                     !ISSET(SOFTWRAP) ?  openfile->edittop->lineno - 
> openfile->current->lineno :
> +                     click_row + (chunk_for(openfile->firstcolumn, TRUE, 
> TRUE, openfile->edittop))
> +                     - (chunk_for(xplustabs(), TRUE, TRUE, 
> openfile->current));

This is horrible.  You can use ?/:, but only when it fits on a single line.
Otherwise you have to use if/else, to have some chance of understanding it.

> -                     original_row = chunk_for(xplustabs(), thisline);
> +                     original_row = chunk_for(xplustabs(), FALSE, FALSE, 
> thisline);

> -                                     chunk_for(xplustabs(), 
> openfile->current) > original_row)) {
> +                                     chunk_for(xplustabs(), FALSE, FALSE, 
> openfile->current) > original_row)) {

Unneeded changes if you had just used a new function elsewhere.

> -     reveal_cursor = showcursor;
> +     /* Hide cursor if it is out of the screen */
> +     reveal_cursor = openfile && openfile->current_y >= 0 && 
> openfile->current_y < editwinrows && showcursor;

Wrong way around: showcursor should come first.

>       if (ISSET(SOFTWRAP)) {
> -             for (linestruct *ln = openfile->filetop; ln != 
> openfile->edittop; ln = ln->next)
> -                     first_row += ln->extrarows;
> -             first_row += chunk_for(openfile->firstcolumn, 
> openfile->edittop);
> -
> -             for (linestruct *ln = openfile->filetop; ln != NULL; ln = 
> ln->next)
> -                     totalrows += ln->extrarows;
> +             first_row += chunk_for(openfile->firstcolumn, FALSE, TRUE, 
> openfile->edittop);
> +             totalrows += chunk_for(0, FALSE, TRUE, openfile->filebot) + 
> openfile->filebot->extrarows;
>       }

Is it necessary to change this?  Is it needed to make dragging work?
If not, don't change it.

> -     for (int row = 0; row < editwinrows; row++) {
> -             bardata[row] = ' '|((row >= lowest && row <= highest) ? 
> A_REVERSE : 0);
> -             mvwaddch(edit, row, COLS - 1, bardata[row]);
> -     }
> +     for (int row = 0; row < editwinrows; row++)
> +             mvwaddch(edit, row, COLS - 1, ' '|((row >= lowest && row <= 
> highest) ? A_REVERSE : 0));

Same thing.  Keep the bardata if it does not hinder dragging.

>  /* Return the row of the softwrapped chunk of the given line that column is 
> on,
> - * relative to the first row (zero-based). */
> -size_t chunk_for(size_t column, linestruct *line)
> + * relative to the first row (zero-based). May add line or chunk number. */
> +size_t chunk_for(size_t column, bool lineno, bool rows, linestruct *line)

??!  Poor choice of parameter names.  Lineno means line number, and thus
one expects it to be a number, not a boolean.  Same for rows.  And the
comment, "May add line or chunk number" is completely unclear -- one has
to read the code to understand what is happening.


>  {
> -     return get_chunk_and_edge(column, line, NULL);
> +     size_t sum = lineno ? line->lineno : 0;
> +     for (linestruct *ln = openfile->filetop; rows && ln != line; ln = 
> ln->next)
> +             sum += ln->extrarows;

Ugh.  You use 'rows' as a hidden if.  :/  It should have been:
  if (rows)
     for (...)

And there should have been a blank line after the declaration of sum.

But you should have made a separate function instead, one that simply
does the required summing without any ifs.  It means you can leave
the eight existing calls of chunk_for() unchanged.

Benno

signature.asc
Description: OpenPGP digital signature