lynx-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: lynx-dev patch - search in partial mode (part1)


From: Klaus Weide
Subject: Re: lynx-dev patch - search in partial mode (part1)
Date: Thu, 18 Nov 1999 12:05:39 -0600 (CST)

On Wed, 17 Nov 1999, Leonid Pauzner wrote:
> 17-Nov-99 12:11 Klaus Weide wrote:
> > I don't like omitting the 'target' parameter from all the calls, for reasons
> > of modularity.  'target' is an important input to display_page() and
> > highlight() etc.  Omitting the parameter makes the dependency less obvious,
> > it hides it.  Making more and more parameters global variables isn't
> > generally a good direction.
> 
> This is not a 'general' case. We have www_search_result global
> which is more obscuring than the prev_target.

Not to mention Newline...  but bad examples don't have to be followed.

> We could leave a 'target' argument in both display_page() and
> highlight() but made prev_target (well, search_target) PUBLIC
> if you think that would be better.

Yes, I think it would be slightly better.

> > You also lose flexibility - there *might* be reason why sometimes
> > display_page() or highlight() should be called without highlighting.
> 
> I am sceptical on the need of changing this argument in display_page()
> outside the mainloop.  Things like "" below was wrong IMHO:

>  PUBLIC void HText_scrollTop ARGS1(
>         HText *,        text)
>  {
> -    display_page(text, 0, "");
> +    display_page(text, 0);
>  }

Those functions are unused anyway, so it's hard to say what would be
right for them...

> > Could you look at this again?  I think you should be able to basically
> > keep 'prev_target' PRIVATE to LYMainLoop.c [but defined outside of
> > mainloop()].  Everything that needs to know it should call functions
> > in LYMainLoop.c or be called from there.
> 
> Currently HText_pageDisplay() called from other places in partial mode
> so we need to keep prev_target somethere if we want search in partial
> mode. 

Only from two places (in HTFormat.c and LYUtils.c) afaics (but I am
looking at the code previous to your patch).  Those partial mode calls
do indeed intrude into LYMainLoop.c territory... (the partial display
code used variables that used to be private to LYMainLoop.c).  IMO
it would be better to replace those HText_pageDisplay() calls with
LYMainLoop_pageDisplay() calls.  LYMainLoop_pageDisplay() would be
implemented in LYMainLoop.c and has access to LYMainLoop.c PRIVATE
variables, it would simply call HText_pageDisplay().

> We could made HText_pageDisplay2() without a search target in
> LYMainLoop.c as an externall call of HText_pageDisplay() but whether
> that would be an improvement?

Ok so you are suggesting the same thing...  I think it would be an
improvement.  If you later find that the partial mode pageDisplay()
calls need to somehow depend on more LYMainLoop.c variables (like
curdoc/newdoc), that can be implemented in one place instead of
two places (HTFormat.c and LYUtils.c) or more.

It seems you are already using a similar approach by calling the
LYMainLoop.c function handle_LYK_WHEREIS() from partial display code.
(Although the public function to use from outside of mainloop() probably
needs to be a bit different...  it shouldn't use curdoc as you have found
out.)

> >> It is interesting enough that handle_LYK_WHEREIS() and textsearch() work
> >> more or less properly while they call '&curdoc' but we load 'newdoc' in
> >> fact. Therefor new line /link position is not saved as it should.
> 
> > Hmm, so it works only more or less by accident...
> a sort of.
> 
> That is because global variable 'www_search_result' is used for new line
> position from textsearch() and HTFindPoundSelector() - the latter is
> responsible for #fragment position.

Well yes, it's quite horrible and confusing.

> Besides that textsearch() also change curdoc.line/curdoc.link

textsearch() doesn't modify curdoc by its global name, it modifies
whatever it gets passed as its first argument...

> Than all that went into Newline changes in mainloop() - that is a mess.
> None of them set inside HText_pageDisplay() currently.
> I am thinking on a kind of unification but have not succeed yet.


  Klaus


reply via email to

[Prev in Thread] Current Thread [Next in Thread]