On Sat, Dec 13, 2008 at 05:20, Martin Friebe <laza...@mfriebe.de> wrote:
> Thanks for the patch, looks good to me.
>
> A few things:
> - Originally you had the condition "NewX1<>NewX2 " moved into
> IsLinkable. Now it is omitted?
> May be right, because codetools may (or may not) return false for
> those. Yet it means an unnecessary call to code tools.
I agree.
> - You probably accidentally included changes to the lfm file of
> source-editor?
Not quite so. The lfm change I included was, IIUC, done by IDE automatically
simply because I opened and saved SourceEditor unit.
This happens often for me, usually since lfm format changed slightly
since the last time anybody touched the lfm in question.
I see three ways to deal with this:
1) Include such changes in commits, since they will inevitably show up later.
2) Ignore them, and let them pile up, and then either
2a) Include them all in bulk when some essential changes to the lfm is needed
2b) Periodically (and automatically, if possible) open and re-save all lfm's
and commit the result.
I do not know what the policy in Lazarus is [judging by my previous
policy questions,
there is probably no policy ;-)]. So I opted for (1).
> - The unlucky bit is that your patch gets affected by a known bug. if
> you got a string with multibyte utf8 chars (like german umlauts) then
> anythink behind the umlauts is not linkable.
>
> This is already the case now (SynEdit does currently not follow valid
> links, behind umlauts), so you patch isn't wrong. The issue is, I can
> not test it, until I fixed the umlauts. I will see, if I can look at
> this during the weekend.
I see that you already committed some fix. FWIW, I tested with Russian
characters in comments -- it works for me.
> - And last not least (not a stopper, but would be nice), the current
> solution does call Codetools each time you move the mouse. It may be
> possible for UpdateControlMouse to cache the boundaries of the current
> linkable word, and trust this cache instead of calling codetools from
> CalculateCtrlMouseLink again?
Good idea, I was concerned with the speed too, but testing showed it to be ok
(maybe my PC is too fast).
So for now I filed an issue http://bugs.freepascal.org/view.php?id=12790
--
Alexander S. Klenin
Insight Experts Ltd.
_______________________________________________
Lazarus mailing list
Lazarus@lazarus.freepascal.org
http://www.lazarus.freepascal.org/mailman/listinfo/lazarus
