[patch] fix another End keypress lockup in the viewer
Pavel Tsekov
ptsekov at gmx.net
Mon Aug 15 08:27:55 UTC 2005
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.
More information about the mc-devel
mailing list