TBaseVirtualTree disable single letter incremental search

[GoranDespalatovic]

Steps to reproduce :

1. Create grid with 1000 nodes, first column filled with record number, enable incremental search
2. Go to node 899
3. Count how many times you have to press key "9" before you reach node 999.

Suggested resolution:

1. add coNoSingleLetterIncSearch to TVTColumnOption
2. add toNoSingleLetterIncSearch to TVTMiscOption
3. in TBaseVirtualTree.HandleIncrementalSearch add :

        // The "single letter mode" is used to advance quickly from node to node when pressing the same key several times.
        if toNoSingleLetterIncSearch in TreeOptions.MiscOptions then            // gigo
          SingleLetter := false                                                 // gigo
        else                                                                    // gigo
        if (FocusedColumn > -1) and (coNoSingleLetterIncSearch in Header.Columns[FocusedColumn].Options) then // gigo
          SingleLetter := false                                                 // gigo
        else                                                                    // gigo
        SingleLetter := (Length(FSearchBuffer) = 1) and not PreviousSearch and (FSearchBuffer[1] = NewChar);

That way we can disable single letter incremental search per column / per tree as needed.

[joachimmarder]

Sorry, but I don't think that the current incremental search behavior is so bad.

Virtual TreeView already has so many options, that they are hard to maintain and hard to understand for new developers. This is why I am very carefully in adding new options.

[GoranDespalatovic]

It's not bad at all. I really like how Virtual TreeView works. Blazing fast, versatile, it's an obvious choice for many tasks.
It's just I've found out that on some sets of data it may be somewhat misleading for user trying to find exact data record and would be nice to have an option to turn this feature off on demand.

First thing I've tried was subclassing TCustomVirtualStringTree, override HandleIncrementalSearch, but some properties and methods used in parent class are private, so I would end up with duplicating a lot of code that already exist in parent class.
Adding 2 options seemed like an easy solution with addition of 10 lines of code and keeping default tree behaviour without breaking anything.

I am aware that Virtual TreeView has (too) many options that could be hard to find for new developers. Maybe we could instead add a property to TVirtualTreeColumn and TBaseVirtualTree, or at least event handler in TBaseVirtualTree.HandleIncrementalSearch in order to enable to switch SingleLetter on/off. That would not disturb option sets for TVirtualTreeColumn and TBaseVirtualTree.

[joachimmarder]

First thing I've tried was subclassing TCustomVirtualStringTree, override HandleIncrementalSearch, but some properties and methods used in parent class are private, so I would end up with duplicating a lot of code that already exist in parent class.

Just let us know what to method this applies and if we need to extract methods from existing. Making methods virtual is typically no problem.

Adding 2 options seemed like an easy solution with addition of 10 lines of code and keeping default tree behaviour without breaking anything.

That's true but you can't do that every week. :-)

It would be good if this could be handled in the OnIncrementalSearchEvent event. What is missing for you to achieve this?

[GoranDespalatovic]

Current implementation is this (in TBaseVirtualTree.HandleIncrementalSearch) :

        SingleLetter := (Length(FSearchBuffer) = 1) and not PreviousSearch and (FSearchBuffer[1] = NewChar);
        // However if the current hit (if there is one) would fit also with a repeated character then
        // don't use single letter mode.
        if SingleLetter and (DoIncrementalSearch(Run, FSearchBuffer + NewChar) = 0) then
          SingleLetter := False;
        SetupNavigation;

we might try to change it to :

  var AllowSingleLetterIncSearch : boolean;


        SingleLetter := (Length(FSearchBuffer) = 1) and not PreviousSearch and (FSearchBuffer[1] = NewChar);
        // However if the current hit (if there is one) would fit also with a repeated character then
        // don't use single letter mode.
        AllowSingleLetterIncSearch := True;
        if SingleLetter and (DoIncrementalSearch(Run, FSearchBuffer + NewChar, AllowSingleLetterIncSearch) = 0) then
          SingleLetter := False;
        SingleLetter := SingleLetter and AllowSingleLetterIncSearch;
        SetupNavigation;

but this would require to add parameter to DoIncrementalSearch method and OnIncrementalSearch event

[joachimmarder]

Changing events in Delphi is always a breaking change, that's the downside. Besides that I like this approach.

[GoranDespalatovic]

How about adding dedicated event like :

        // However if the current hit (if there is one) would fit also with a repeated character then
        // don't use single letter mode.
        SingleLetter := DoAllowSingleLetterIncrementalSearch(Run);
        if SingleLetter and (DoIncrementalSearch(Run, FSearchBuffer + NewChar) = 0) then
          SingleLetter := False;
        SetupNavigation;

with

TVTAllowSingleLetterIncrementalSearchEvent = procedure(Sender: TBaseVirtualTree; Node: PVirtualNode;
    var Result: Boolean) of object;

and

function TBaseVirtualTree.DoAllowSingleLetterIncrementalSearch(Node: PVirtualNode): Boolean;
begin
  Result := True;
  if Assigned(FOnAllowSingleLetterIncrementalSearch) then
    FOnAllowSingleLetterIncrementalSearch(Self, Node, Result);
end;

[joachimmarder]

Well, the same applies here like for the TreeOptions: You can't add an event for any edge case. I still like the additional parameter in the existing event more.

[joachimmarder]

but this would require to add parameter to DoIncrementalSearch method and OnIncrementalSearch event

I will accept a pull request for this for V8, which already includes a few breaking changes.

[MHumm]

Maybe the parameter could have a default value so it is optional and the breakage thus is minimized?

[joachimmarder]

Default parameters don't help for events if the old code doesn't expect this new parameter. It's a common problem in Delphi, too much to explain here.