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! _______________________________________________ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel