SynEdit AutoCompete, prepare ItemList before Execute

[Zaher]

TSynBaseCompletion.Execute procedure(s: string; x, y: integer), Lines 1397 In file
components\synedit\syncompletion.pas

Code: [Select]

      CurrentString := s;
      if Assigned(OnExecute) then
          OnExecute(Self);

Here CurrentString sets before OnExecute, while i am filling ItemList on OnExecute, that makes it lost the position of current item.

Where I can fill the list every time execute the autocomplete? is there any event or function to override?
If not is it bad idea to do

1 - move CurrentString := s; after on execute, but that will make CurrentString empty, not good idea
2 - make new event OnPrepare we can call it before set CurrentString
3 - Make Execute method as virtual so i can easily inherit

Thanks in advance

[Zaher]

Or by adding new method called by procedure Execute(s: string; x, y: integer);

protected procedure InternalExecute(s: string); virtual;

Can I make this patch?

[Martin_fr]

Did you look at the example? C:\lazarus\examples\SynEdit\Completion\

Maybe SynCompletion1.OnSearchPosition := @DoSearchPosition ?

[Zaher]

thanks, but the problem also exists in that example
Run program, remove all checks, go to "co" word and press ctrl+space, first time  it shows wrong position.

[Martin_fr]

OnSearchPosition
is the correct answer (but only, if it also does the filling of the list)

DoSearchPosition
Is a bad example.

And the checkboxes are horrible wrong.

"Filter in Search Position" Does enable/disable the entire event.

So set "Filter in Search Position" to checked.

Then look at search position. It currently always returns 0 (or -1).
It should return the index with the best match instead.

--------------------------------------
But I see what you mean.
This lets OnExecute unusable for the purpose of filling the list.

Yet we need to set the CurrentString first, because user code may have OnExecute events, that relay on it.

I would say something like

Code: Pascal  [Select][+][-]

1.   CurrentString := s;
2.   if Assigned(OnExecute) then begin
3.     OnExecute(Self);
4.     CurrentString := s;
5.   end;
6.  

Though that calls OnSearchPosition twice.

So better

Code: Pascal  [Select][+][-]

1.   if Assigned(OnExecute) then begin
2.     CurrentStringDirect := s; // do not adjust position / better name needed
3.     OnExecute(Self);
4.   end;
5.   CurrentString := s;
6.  

---------------
OnExecute may  have called SetPosition. (that must be added to the list of incompatible changes.)

I think it is ok to ignore SetPosition calls from OnExecute. Since there is a dedicated event for this / and the pos from OnExecute is lost as soon as you keep typing.

[Martin_fr]

report on mantis please.

feel free to cleanup/complete the patch. thanks.

[Zaher]

I think there is 2 ways, first one will break back-legacy

OnExecute(Self, s);

Or adding new event OnPrepare but I think OnExecute made for same propose.

also adding CurrentStringDirect  will break old projects because CurrentString will be empty.

or another idea setting FCurrentString direclty  then call SetCurrentString(s)

[Martin_fr]

Code: Pascal  [Select][+][-]

1. OnExecute(Self, s);

Changing an event signature is no good, not sure that may even break lfm loading.

"CurrentStringDirect" is not a new property.
It will be a private method (avail between classes in that unit), and it will set "CurrentString", but not call the position event (also not do the internal calc for position.

So old code in OnExecute, that is reading "CurrentString" will get the expected result.

But the real call to "CurrentString" (that triggers calculating the position) is done after OnExecute (as you proposed)

--------
As I set "CurrentStringDirect"  needs a better name, maybe
procedure InternalSetCurrentString(s:string);

Form.InternalSetCurrentString(a)  will be the same as Form.FcurrentString:=a.
But the latter is bad, as it access private members of another object.

Code: Pascal  [Select][+][-]

1.   if Assigned(OnExecute) then begin
2.     Form.InternalSetCurrentString(s); // Form.FcurrentString:=s / for old code
3.     OnExecute(Self); // execute before position is calculated.
4.   end;
5.   CurrentString := s; // call OnSearchPosition // or do internal pos
6.  

[Bart]

Code: Pascal  [Select][+][-]

1. OnExecute(Self, s);

Changing an event signature is no good, not sure that may even break lfm loading.

Even worse, the IDE will give no warning that the signature is wrong, compilation will NOT fail and you program will crash, but you won't know why.

See: http://bugs.freepascal.org/view.php?id=23032.

So, please, pretty please do not do that.

Bart

[Zaher]

Bug reported with patch

http://bugs.freepascal.org/view.php?id=30965