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

[Pavel Tsekov <ptsekov gmx net>]

Hello,

On Sun, 14 Aug 2005, Jindrich Makovicka wrote:

> the new viewer has a problem with viewing files with DOS line
> separators, which manifests again in a lockup after pressing the End
> key. Then, it is impossible to move the view further, except by pressing
> the Home key.
>
> It is caused by the code of view_move_up, which moves by one line using
> view_coord_to_offset() and then view_offset_to_coord() with offset
> decremented by one. In the case of 0d/0a separator, the decremented
> offset points to 0d, and view_offset_to_coord() returns the same coords.
>
> The attached patch fixes the issue by decrementing the offset until
> something else than 0d appears.

I've also been looking into this issue lately but I haven't got enough
time to write a proper patch and I wanted to discuss this issue with
Roland. Now looking at your patch I think that your approach is not the
best one. Before commiting this patch consider the facts below.

The actual problem does not lie in view_move_up() but in the viewer's
coordinate cache handling. view_ccache_lookup() doesn't do proper handling
for '\r' and as result the same offset in the file is returned for both
'\n' and '\r' thus the code in view_move_up() cannot perform the move in
the case described by Arpad Biro. I think fixing view_ccache_lookup() is
how the problem should be attacked.

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. 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 - line, 0);

I haven't investigated this possibility properly but maybe Roland can
comment.