Re: [PATCH] Help Viewer - incorrect behaviour of <up> arrrow key

[Pavel Tsekov <ptsekov gmx net>]

Hello Grigory,

On Mon, 4 Dec 2006, Grigory Trenin wrote:

> Hello Pavel,
>
> Pavel Tsekov wrote:
>
> > I think the correct solution would be to keep both
> > 'startpoint' and 'currentpoint' in the history but I am
> > open to suggestions.
>
>
> Yes, I thought about it too. This is a good and safe fix.
> But I have a suggestion.
> Actually, 'startpoint' is used only once at line 315 of help.c:
>
>   p = current_link - 1;
>   if (p <= start)
>       return 0;
>   p = search_char_node (p, CHAR_LINK_START, -1);
>   return p;
>
> Here 'current_link' is the pointer to the currently
> selected (highlighted) link, and 'start' == 'startpoint'.
> Since there can be no situation when currently selected link
> is outside of the current topic, (p <= start) expression
> becomes true only in one rare case - when the first text in the node
> is a link, for example:
> ^D[Topic name]^ASome link^BSome link^C
>
> In that case 'p' will point to ']' character, and 'start_point' will
> point to ^A, so (p <= start) condition will be true. But
> this check for (p <= start) condition is not really necessary,
> because search_char_node() will not search beyond the topic
> boundaries, it will stop at ^D character and return NULL.
>
> So, if that check and the 'startpoint' variable itself are not
> really necessary and can be eliminated, why should we mess with
> saving/restoring 'startpoint' in the history?
>
> Please look at a new patch - what do you think?
> There I removed 'startpoint' as well as the first argument of
> select_prev_link() function. And after calling it there is no sense to
> check if (selected_item >= last_shown) - this will never happen,
> so I removed it also.

I am not convinced that this patch is better than your first
one. Its basically the same and removes a valid check - it
doesn't fix the issue.


Try the following:

COLS = 126
LINES = 60

1) F1 (Help)

2) Select "contents" and hit "Enter"

3) Press "Page Down"

   You should end up on "Executing operating systems commands"

4) Press "Up"

You should notice that the marker doesn't select the
previous link as it should. You need to press "Up"
one more time.

P.S. Sorry for the late reply but I have some problems
which I need to take care of.