Re: [patch] fix another End keypress lockup in the viewer

[Roland Illig <roland illig gmx de>]

Pavel Tsekov wrote:
> Another approach would be to simplify the following code from
> view_move_up():
>
>             } else if (line >= 1) {
>                 view_coord_to_offset (view, &linestart, line, 0);
>                 view_offset_to_coord (view, &line, &col, linestart - 1);
>
>                 /* if the only thing that would be displayed were a
>                  * single newline character, advance to the previous
>                  * part of the line. */
>                 if (col > 0 && col % width == 0)
>                     col -= width;
>                 else
>                     col -= col % width;
>             } else {
>
> It is clear now that the assumption that `linestart - 1' is not always an
> offset inside the text on the previous line.

EPARSE. Did you mean this:?

> It is clear now that `linestart - 1' is not always an offset inside
> the text on the previous line.

Why? If only because of the buggy coordinate cache, this does not count. 
The bug is fixed.

> Is it safe to assume that if `line' does exist `line - lines' would
> also exist (of course if lines is not greater than line) ? If so why
> don't we just do:
>
>  view_coord_to_offset (view, &(view->dpy_topleft), line - lines, 0);

Because we are in text wrap mode. Internally the viewer stores the 
"unwrapped" coordinates. When you want to go one line up, you have to 
distinguish whether you are inside a long line or at the beginning of a 
line. This is what the code does.

The code you suggest is used in the !hex_mode && !text_wrap_mode case.

To my knowledge, the code cannot be made simpler. I could add some 
comments to the code to make understanding it simpler. Shall I?

Roland