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

[Pavel Tsekov <ptsekov gmx net>]

Hello Grigory,

On Mon, 2006-12-04 at 19:45, Grigory Trenin wrote:

> 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.

Ok. I think your arguments are reasonable. If the help viewer code was
better organized I'd probably have insisted on keeping the variable
holding the start of the node. However, I think that currently
'startpoint'
brings more confusion than benefit, so I've applied your patch.

Thanks!