Lazarus

Using the Lazarus IDE => Editor => Topic started by: Paolo on September 21, 2020, 07:13:02 pm

Title: Search again
Post by: Paolo on September 21, 2020, 07:13:02 pm
hello, in delphi once a search is done, for example for a variable "x", all the found occurence of "x" are shown, and the same in lazarus. But in delphi if I modify or remove some "x in the code it is possible right click on the previous search result, very easy an clear, to update the search content, in lazarus I did not find such function and this means that after several code editing and search the message box is populated of the several tabs search result and the message list becomes ureadable. Can I achieve the same functionality as in Delphi ? Thank you
Title: Re: Search again
Post by: JuhaManninen on September 21, 2020, 09:36:55 pm
You can freely close any tabs in the Search Results window. In Lazarus trunk (development version) it even has buttons to close left, other, right or all tabs.
Title: Re: Search again
Post by: Paolo on September 21, 2020, 10:51:22 pm
Thanks for the reply.

I think the delphi approach is more consistent.
I'll try to compare the lazarus way to delphi way.
Delphi
1) search, for example for X var,
2) the search result is showing the found Xs occurrences in the code
3) jump from one of X in search result to the code line
4) do the job, now the search result need to be updated
5) right click on the search result of X, on the popup do search update
6) repeat steps 3) to 5) as many times as needed, in the serch result list there is just one tab for X.
If you are working alternating on more variables, just go to the relevant tab and do search again, without risk of duplicated tabs around.

Lazarus
1) search, for example for X var,
2) the search result is showing the found Xs occurrences in the code
3) jump from one of X in search result to the code line
4) do the job, now the search result need to be updated
5) close the previous X search result
6) reopen the search dialog form, and press ok
7) repeat steps 3) to 6) as many times as needed, but..
If you forget step 5) you have a duplicated tab with different results
If you are working alternating search among more variables at step 6) you have to retype the var (or select it from the list of previously searched.

I think the delphi approch is clearer and more coherent.
Title: Re: Search again
Post by: Martin_fr on September 21, 2020, 11:32:47 pm
I agree, it is a "nice to have".

For now, all there is, is the long way.

You can make it a feature request, but I do not know when (or even if) someone would pick it up...
Otherwise it is "patch welcome"

I don't know your skill level, but it may take less than you expect. If you are willing to put in the hours.

Each  new search is triggered from main.pp line 10886 (lazarus svn trunk)
   function TMainIDE.DoFindInFiles: TModalResult;

Units involved
  ide\findinfilesdlg.pas
  ide\SearchFrm.pas  // TSearchProgressForm
  ide\SearchResultView.pp  // TSearchResultsView


Look at TSearchProgressForm.
Any of the DoSearch*** methods end up calling DoSearchAndAddToSearchResults

Then you can modify the SearchResultsView add the contexct menu.
Make sure you have *ALL* the search options, in order to repeat the search, and re-use the existing tab.

In fact
procedure TSearchResultsView.SearchAgainButtonClick(Sender: TObject);
already shows you how to get the old search options.
But it either does not yet store all, or does not restore them all to the dialog (that could be fixed too). Also You want to skip the dialog.



It's only 2 (maybe 3) units that need to be looked at. So it does not require any deep knowledge. And also, there is no shame in asking further questions. If you want to give it a go, I am glad to answer questions.


Title: Re: Search again
Post by: JuhaManninen on September 23, 2020, 01:17:16 am
+1
Yes, actually it is a good idea.
Title: Re: Search again
Post by: Paolo on September 23, 2020, 06:02:49 pm
I think to try both:

1) How can I submit a "requested feature" ?
2) I don't know if I'll be able to accomplish the "patch" task, but I would try....

Thanks
Title: Re: Search again
Post by: Martin_fr on September 23, 2020, 06:27:27 pm
Feature request => on the bugtracker.

Patch => start looking at the code. If you have questions: ask.
Title: Re: Search again
Post by: Paolo on September 26, 2020, 05:35:54 pm
Dear Martin_fr,

I started to modify the code, but before add "repeat search", I thought to modify two other steps (agian in the delphi style):

1) if search give "0" items found the searched string is not shwn in the tablist and a message is informing the user of failed search.

2) if the search was already done on the word "abc", the old tab is deleted and the new one shown.

Apparentlly I was succesful in doing both the above tasks... I'm testing, then I would like to share with you what  I ave done (maybe  I did something violating some coding rules in "Lazarus" team).

After that  I'll try to add the "repeat search" functionality.

From what  I see I have the feeling that code for pure search is mixed with code for showing results, so it is difficult to extrapolate only the search part, but I have just given alook at the code....

as soon as possible I'll show you what  I have done, maybe not so elegant but quite striaghtforward.

Ciao.
Title: Re: Search again
Post by: Martin_fr on September 26, 2020, 06:31:35 pm
First of all: If you do more than one change try to make a separate patch for each changeset.

If you want to use git (which easily allows for this), you can clone my mirror at https://github.com/User4martin/lazarus
For the specific changes in regards to this thread you may create pull requests. (But patches are equally fine).

Pull requests for any other feature, not part of the search related changed, must be pre-agreed for pull requests.

I started to modify the code, but before add "repeat search", I thought to modify two other steps (agian in the delphi style):

1) if search give "0" items found the searched string is not shwn in the tablist and a message is informing the user of failed search.

2) if the search was already done on the word "abc", the old tab is deleted and the new one shown.
I am not much in favour of those two.

1) A search may take rather long. It is possible to carry on working during the search (I have done that). In such a case, I do not want to be interrupted by an alert about a failed search, as this would take the focus from the editor.
In that sense, I also much prefer, that the search result window opens when the search starts. So when the search ends, nothing takes away the focus.

2) It is ok, only if the search settings are 100% identical.
I may (and have in the past) search(ed) the same term, but with different settings (whole word or not / different dir).
I could see an option where they get grouped as different tree nodes into the same tab. That is, instead of having the directory tree nodes, at top level, there would be a new top level per search. (with some useful info, how the 2 or more searches differed).
That however is a lot more work.


Quote
Apparentlly I was succesful in doing both the above tasks... I'm testing, then I would like to share with you what  I ave done (maybe  I did something violating some coding rules in "Lazarus" team).
You can submit them, then I will review them.

Need to see, if the actual changes are desirable in the current proposed way.
Title: Re: Search again
Post by: JuhaManninen on September 26, 2020, 10:24:16 pm
I am not much in favour of those two.
I agree.
1) Same reason as Martin has.
2) This does the same thing as the original "search update" feature but is less intuitive.
It is better to stick with the original plan. If a user wants to reuse the same old tab, he chooses "search update". Otherwise he does a new search with a dialog.
This is flexible and offers more choices.
Title: Re: Search again
Post by: Paolo on October 05, 2020, 01:32:23 pm
Hello,

here my first step attempting to follow the original plan (JuhaManninen : "It is better to stick with the original plan")

The plan is:

user do a search with some selected options
then
a) it is a new search, and a new tab is added to the search result
b) the search has been already done (same text, same options), here we can follow to path:
b1) update a previous search
b2) remove the old search and add a new tab.

I selected the b1) that seems has less impact on the already availabe code and offers some other advantages..

But at this stage the first step was that even the user go through "find in file dialog" the behaviour has to be as above described, then will follow the other part of the code in case of just "search again" on the relevant tab (to be completed).

Here my proposal and code modification of fisrt stage of this topi when the user goes through the dialogform:

In UNIT SEARCHESULTVIEW
  TSearchResultsView = class(TForm)
  ..
  public
     function IfAlreadySearched(Index : integer; const SearchText : string): boolean;
   ..
   end;

the code implementation


function TSearchResultsView.IfAlreadySearched(Index : integer; const SearchText : string): boolean;
var
  CurrentTV: TLazSearchResultTV;
  SearchObj: TLazSearch;
begin
  result:=false;
  CurrentTV:= GetTreeView(Index);   
  SearchObj:= CurrentTV.SearchObject;
  if SearchObj.fSearchString = SearchText then begin
    result:=(SearchObj.fSearchOptions = FindInFilesDialogSingleton.Options);
  end;
end; 

In UNIT SEARCHFRM

procedure TSearchProgressForm.DoSearchAndAddToSearchResults;
var
  ListPage: TTabSheet;
  Cnt: integer;
  State: TIWGetFormState;
  i : integer;
begin
  Cnt:= 0;
  LazarusIDE.DoShowSearchResultsView(iwgfShow);

//here the "only" added block, if the same occurence of search is found it is deleted
//It could be (in the future imic the curretn lazarus behaviour) a user option to have separate tabs result for same results, so it is
//better go through all the results
  i:=0;
  while i < SearchResultsView.ResultsNoteBook.PageCount do begin
    if SearchResultsView.IfAlreadySearched(i, SearchText) then
      SearchResultsView.ClosePage(i)
    else
      i:=i+1
   end;

    ListPage:=SearchResultsView.AddSearch(SearchText,SearchText,
                                ReplaceText,SearchDirectories,SearchMask,SearchOptions);
      try
        (* BeginUpdate prevents ListPage from being closed,
          other pages can still be closed or inserted, so PageIndex can change *)
        SearchResultsView.BeginUpdate(ListPage.PageIndex);
        ResultsList:= SearchResultsView.Items[ListPage.PageIndex];
        ResultsList.Clear;
        ResultsWindow:= ListPage;
        try
          Show; // floating window, not dockable
          Cnt:= DoSearch;
        except
          on E: ERegExpr do
            IDEMessageDialog(lisUEErrorInRegularExpression, E.Message,mtError,
                       [mbCancel]);
        end;
      finally
        ListPage.Caption:= Format('%s (%d)',[ListPage.Caption,Cnt]);
        SearchResultsView.EndUpdate(ListPage.PageIndex);
        // show, but bring to front only if Search Progress dialog was active
        if fWasActive then
          State:=iwgfShowOnTop
        else
          State:=iwgfShow;
        LazarusIDE.DoShowSearchResultsView(State);
      end;
end;

Dear Martin_Fr and JuhaManninen comments are welcome (maybe I am very far from the "right solution"). If it is ok we can move on the search again without passing through dialog box.
Title: Re: Search again
Post by: lucamar on October 05, 2020, 04:48:34 pm
No comment about the feature (it's one I rarely use) but please, Paolo, read this section in the wiki: "Use code tags (https://wiki.freepascal.org/Forum#Use_code_tags)" and follow it; it makes easier for the rest of us to read code in posts like yours. Thanks! :)
Title: Re: Search again
Post by: Paolo on October 05, 2020, 04:56:46 pm
thanks a lot lucamar, here the more readable version

Code: Pascal  [Select][+][-]
  1.  
  2. In UNIT SEARCHESULTVIEW
  3.  
  4.   TSearchResultsView = class(TForm)
  5.   ..
  6.   public
  7.      function IfAlreadySearched(Index : integer; const SearchText : string): boolean;
  8.    ..
  9.    end;
  10.  
  11. the code implementation
  12.  
  13.  
  14.  
  15. function TSearchResultsView.IfAlreadySearched(Index : integer; const SearchText : string): boolean;
  16. var
  17.   CurrentTV: TLazSearchResultTV;
  18.   SearchObj: TLazSearch;
  19. begin
  20.   result:=false;
  21.   CurrentTV:= GetTreeView(Index);  
  22.   SearchObj:= CurrentTV.SearchObject;
  23.   if SearchObj.fSearchString = SearchText then begin
  24.     result:=(SearchObj.fSearchOptions = FindInFilesDialogSingleton.Options);
  25.   end;
  26. end;
  27.  
  28. In UNIT SEARCHFRM
  29.  
  30. procedure TSearchProgressForm.DoSearchAndAddToSearchResults;
  31. var
  32.   ListPage: TTabSheet;
  33.   Cnt: integer;
  34.   State: TIWGetFormState;
  35.   i : integer;
  36. begin
  37.   Cnt:= 0;
  38.   LazarusIDE.DoShowSearchResultsView(iwgfShow);
  39.  
  40. //here the "only" added block, if the same occurence of search is found it is deleted
  41. //It could be (in the future imic the curretn lazarus behaviour) a user option to have separate tabs result for same results, so it is
  42. //better go through all the results
  43.   i:=0;
  44.   while i < SearchResultsView.ResultsNoteBook.PageCount do begin
  45.     if SearchResultsView.IfAlreadySearched(i, SearchText) then
  46.       SearchResultsView.ClosePage(i)
  47.     else
  48.       i:=i+1
  49.    end;
  50.  
  51.     ListPage:=SearchResultsView.AddSearch(SearchText,SearchText,
  52.                                 ReplaceText,SearchDirectories,SearchMask,SearchOptions);
  53.       try
  54.         (* BeginUpdate prevents ListPage from being closed,
  55.           other pages can still be closed or inserted, so PageIndex can change *)
  56.         SearchResultsView.BeginUpdate(ListPage.PageIndex);
  57.         ResultsList:= SearchResultsView.Items[ListPage.PageIndex];
  58.         ResultsList.Clear;
  59.         ResultsWindow:= ListPage;
  60.         try
  61.           Show; // floating window, not dockable
  62.           Cnt:= DoSearch;
  63.         except
  64.           on E: ERegExpr do
  65.             IDEMessageDialog(lisUEErrorInRegularExpression, E.Message,mtError,
  66.                        [mbCancel]);
  67.         end;
  68.       finally
  69.         ListPage.Caption:= Format('%s (%d)',[ListPage.Caption,Cnt]);
  70.         SearchResultsView.EndUpdate(ListPage.PageIndex);
  71.         // show, but bring to front only if Search Progress dialog was active
  72.         if fWasActive then
  73.           State:=iwgfShowOnTop
  74.         else
  75.           State:=iwgfShow;
  76.         LazarusIDE.DoShowSearchResultsView(State);
  77.       end;
  78. end;
  79.  
Title: Re: Search again
Post by: Martin_fr on October 05, 2020, 05:41:49 pm
Code tags is fine. But for changes to existing code, it needs to be a patch/diff.
https://wiki.lazarus.freepascal.org/Creating_A_Patch

Title: Re: Search again
Post by: Paolo on October 05, 2020, 07:37:18 pm
As first step I have to download GIT (is it this one ? https://git-scm.com/)
Title: Re: Search again
Post by: Martin_fr on October 05, 2020, 08:00:02 pm
git is one way.
The link is fine...:. (I use https://gitforwindows.org/ but I think it links to the same installer)

BUT:

On Windows you should consider TortoiseSvn or TortoiseGit => makes live easier (And I think you do not need the above in that case.



However it is possible without git too / though you would base the patch on an older file. But for smaller patches that may work. (I.e. chances are the patch will apply to the current trunk version too)

Then all you need is:
- keep the original file
- have the new file
- use "diff oldfile newfile" (On Windows from the fpc dir / On linux should be part of the OS)



If you go git and/or svn.

If you download the latest source from either git or svn then you can create a 2nd installation (that will not interfere with your stable install)
Create a lazarus.cfg file in the svn/git checkout https://wiki.lazarus.freepascal.org/Multiple_Lazarus#Using_lazarus.cfg_file
Copy your existing Lazarus.exe over, and rebuild from inside the IDE.
=> Currently that will complain about a missing/duplicate package. You need to install the package, and then when you get the error edit out the refernce in the main.pp or lazarus.pp file. // Or run lazbuild (there should be a wiki on that) // Or ask and I sent you a prebuild lazarus.exe that will get you going (assuming win64 or win32)

Title: Re: Search again
Post by: Paolo on October 07, 2020, 03:49:23 pm
Hello,

there is something "funny" :D in the explanation of "how to submit a patch".

I followed the path:

1. https://wiki.lazarus.freepascal.org/Creating_A_Patch
2. then there is written "The recommended way to submit a patch is through the bug tracker, see How do I create a bug report for details"
3. now I am in "https://wiki.lazarus.freepascal.org/How_do_I_create_a_bug_report"
4. where it says :"If you have a possible solution, you can add a patch - see Creating A Patch, which will speed up the process."
 :o

In the mean time I have also tried the command:

diff oldSearchfrm.pas newSearchfrm.pas > diffSearchfrm.pas
(did the same for the other file)

it works, in the output file there is the difference, but then ? have I to subscribe to bugtracker ? and submit what ? the two difference files with what name ?

In any case I will try Tortoisesvn

Sorry for these trivial questions, but for me it is a completely new field.




Title: Re: Search again
Post by: Martin_fr on October 07, 2020, 09:53:41 pm
In the mean time I have also tried the command:

diff oldSearchfrm.pas newSearchfrm.pas > diffSearchfrm.pas
(did the same for the other file)

it works, in the output file there is the difference, but then ? have I to subscribe to bugtracker ? and submit what ? the two difference files with what name ?
For now you  can submit the patch in here in the forum, as attachment.
This is because we are already discussing it, and agreed to review it.

Patches on the forum only make sense, if you know that someone is ready to take them from them.

The bugtracker helps ensuring a patch will not be lost, if no one can react immediately.
The bugtracker is also sometimes used to keep information about why a patch was done.

Rule of thumb: Use bugtracker, unless a developer said otherwise (which is the case here).

Quote
Sorry for these trivial questions, but for me it is a completely new field.
No problem at all.
Title: Re: Search again
Post by: JuhaManninen on October 07, 2020, 11:54:37 pm
diff oldSearchfrm.pas newSearchfrm.pas > diffSearchfrm.pas
No. You should do :
$ svn diff > mypatch.diff
You must have Lazarus trunk from SVN server. One patch can have changes for many source files.
Title: Re: Search again
Post by: Martin_fr on October 08, 2020, 02:03:48 am
diff oldSearchfrm.pas newSearchfrm.pas > diffSearchfrm.pas
No. You should do :
$ svn diff > mypatch.diff
You must have Lazarus trunk from SVN server. One patch can have changes for many source files.

Once he has svn.

If he does not, normal diff takes both files.
But yes normal diff also benefits from >mypatch.diff
Title: Re: Search again
Post by: JuhaManninen on October 08, 2020, 11:27:05 am
If he does not, normal diff takes both files.
But yes normal diff also benefits from >mypatch.diff
Any substantial patch should be made against trunk (development version) code. Otherwise the chance for merge conflict is high.
The easiest way to get Lazarus trunk code is by SVN or Git tool. Thus, better get it right away.
With Git you make local commits and then generate a patch with "git format-patch", or create a pull request for Martin's GitHub repository.
Title: Re: Search again
Post by: Paolo on October 17, 2020, 11:40:45 am
Hello,

using tortoisesvn on downloaded trunk version, attached the file , is it fine to be submitted for a patch ?
Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 01:25:53 pm
Yes, this is a proper patch.


About the content.

This change still "replaces" the last result for the same search term.
Actually it does not even replace it. Because it adds a new tab at the end of the list of tabs, and closes the old one => changing the order.


IMHO here is what should happen.

There should be a refresh button on the search-result-form. (Or a popup menu to the existing tabs, with a refresh entry / or both).
Then the search should be repeated. And the existing tab re-used in place.


The setting for  the search are already stored on the treeview.SearchObject

So from a quick glance

- Add the button
- Find the current tab, and the search settings
- Call a modified 
      procedure TSearchProgressForm.DoSearchAndAddToSearchResults;
         (ListPage: TTabSheet = nil)
  to which you pass the existing
       ListPage
  so it does NOT need to do
      ListPage:=SearchResultsView.AddSearch


I may have overlooked something, but in essence that should be it.


There are lot of other optional changes that could improve/extend the search. But the need to be discussed before getting implemented.

---
I am going to be away for 2 or 3 weeks / response times may longer.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 01:59:47 pm
IMHO here is what should happen.

There should be a refresh button on the search-result-form. (Or a popup menu to the existing tabs, with a refresh entry / or both).
Then the search should be repeated. And the existing tab re-used in place.
+1
That was my idea, too.
I don't know how Delphi implemented it. Their design is often not perfect. Lazarus IDE does not intend to be a clone of Delphi although LCL is supposed to be compatible with VCL.
I will follow this thread for questions or new patches.

P.S.
Oh boy Martin, wouldn't "666" be enough. :)
Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 02:48:45 pm
Other options that I would find acceptable:

1) A context-menu (on the tab) could have the entries: "refresh" and "Modify search". "update" would redisplay the search dialog, allowing to modify the search (change directory, and even search term) => and then re-use the tab.
The search dialog would then have a caption "Modify search" or similar to indicate what is happening.

2) Using the extended notebook (as used by source editor) and allow drag/drop changing the order of tabs.



I do not know how important it is, to replace an existing tab with a new search....

I.e. from your current approach I can see, that you would like to be able to select "Search in files" (keyboard shortcut or menu), and then if the search-term is the same as a previous replace the previous result-tab (keeping the tab order).

I have no problem with that idea in general. BUT, it should not be the default behaviour. It must in some way be optional.
And that is, where it becomes a bit tricky.

A)
The "search in files" dialog itself is already quite crowded. Over the past, many people had objections to extending it. (Hence it is also kept separate from the normal "search" dialog // I.e. not like notepad++ which combines those dialogs).
So that speaks against the idea to add a checkbox to that dialog, which would be the easiest way to make it optional.

B)
A global option (Tools > Options) "Allow Search in files to re-use tabs for matching search terms".
However that means you set the option once, for all your searches. That is quite limiting.

So then I can think of 2 ways to allow that feature
* A global option "Show extended search in files dialog"
  => And then we can have the "replace existing tab" checkbox (or even dropdown) in the dialog.
  And that also allows other extensions in future.
  And yet by default the "search in files" dialog remains as it is.
* Create a new "Search in files" dialog, in a package.
   => If the package is installed, it will replace the existing dialog.
  But that is considerable more work. And requires more insight into the IDE code.


Maybe there are other ideas....

IMHO global option "Show extended search in files dialog" is the way to go, if new searches should be able to replace existing ones.
Adding the global option is quite easy too.



In any case I would recommend a step by step approach.

The first part should be to get the "refresh button" (or context-menu on the tab / your choice).
Title: Re: Search again
Post by: Paolo on October 17, 2020, 03:14:44 pm
Dear Martin_fr and JuhaManninen,

I attached a doc with the implemented functionality in "action", and the doc explains what is the logic (on a very basic example), anyway:

1)   at the first stage the current patch is about user input done through the search dialog box, it has to avoid duplicated identical search. On the matter if the new search has to replace the previous one or appear a new tab can be reviewed (even if I have the impression that the here adopted logic is less invasive wrt the available code implementation and I like that the new search is put in evidence in the last tab, I admit this are personal preferences).
2)   the second, and the main objective of the patch, is illustrated in the doc. The user right click on the active search tab and a Pop up allow to repeat the same research (I would like to spend some time more to check the code) but basically it is already implemented (and work in conjunction with patch at point 1), for this reason at first step I would submit just the first patch, the second shall follow.

thank you supporting me in this activity.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 03:19:19 pm
Please keep it simple. IMO there are too many options already, some of them confusing.
Why new searches should replace existing ones? The tab is already there, just use it. Logical and intuitive from a user's point of view, too.
A related improvement :
Quote
Using the extended notebook (as used by source editor) and allow drag/drop changing the order of tabs.
Yes, I have already missed the ability to reorder the tabs. How much work would it be? I guess not much because the component already supports it.
Title: Re: Search again
Post by: Paolo on October 17, 2020, 03:21:23 pm
attached file with some typos corrected
Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 03:46:51 pm
Please attach only images, and keep text readable in the forum.  You can attach up to 4 images.

Quote
By the first patch here proposed, multiple result (same text and options) not allowed, that is clearer
showresult form now

1) That is not exactly what the patch does. It does not compare the directory, nor the file mask.
I do occasionally search the same search, but in 2 or more different directories. Then I need both results (though having them in one tab would in some cases be convenient)

2) The patch does not make it optional.
I have actually sometimes a need to keep the old search, despite it being identical.
A typical scenario is during refactoring. Where I rewrite (not just simple replace, but replace with a variety of different code snippets) some code. I.e. I search for "GetMainFormSizeAndPos" and I replace it (and surrounding code) with different new code.
- I do want to keep a list of all the locations that I changed (I.e. the original search result). So I can re-visit any code I already changed.
- I will re-run the search to check if I missed any location
=> I need both results.


So having duplicated tabs is actually a feature that I do not want to remove.
A patch must therefore be an extension to the feature, allowing the user to chose.
See my other post, on how such an option could be done. (And maybe @Juha has some ideas?)

There are a lot of people using the IDE. In my time working on it, I have countless times been surprised how other people may be using a feature that I would have considered unusable.




As for adding an option. My gut feeling is to best add it to Editor > General > Miscellaneous
ide/frames/editor_general_misc_options
You need to look at: Setup, ReadSetting, WriteSettings

In TEditorOptions just add a published property. That will be saved automatically.

Then you can extend the "search in files" dialog, and hide/show extra controls based on the new setting.


@Juha: Any idea if that could be simplified?
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 03:48:29 pm
@Paolo, looking at your ide.patch I dare to say it is flawed. It only compares the SearchText, not the other search criteria. A user would want to search from different directories, using case sensitive/insensitive, regular expression etc., and have their results in separate tabs.
Your patch destroys that!

The whole case a) in your PDF is over-engineering IMO. People have different work flows and motives. They should be able to have tabs with fully identical search criteria if they want.
For example somebody may search after updating sources from revision control, or search files in a network drive, and compare the results with an earlier search.
Your solution tries to be "smarter than the user" which is a doomed approach.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 04:01:25 pm
@Juha: Any idea if that could be simplified?
Just leave any options out at least now. It would add complication for very little benefit.
Better and more intuitive would be to support reordering the tabs by user. Is the current PageControl (or something?) easy to replace with the extended notebook? You have best knowledge of it.

The "Search again" in Paolo's popup menu is good. Even better would be to have it in the tab itself, in addition to the TreeView under the tab.
Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 04:12:35 pm
Better and more intuitive would be to support reordering the tabs by user. Is the current PageControl (or something?) easy to replace with the extended notebook? You have best knowledge of it.
Its a drop in replacement. Should be possible to use the object-inspectors "change class" feature.
It would then need to handle the OnTabDragDrop event (and maybe other OnTab.... events) to make sure the treeviews are correctly associated (not sure how much work is needed there).
Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 04:15:57 pm
The "Search again" in Paolo's popup menu is good. Even better would be to have it in the tab itself, in addition to the TreeView under the tab.
Thats why pdf attachments are not a good idea.
I did not even notice the 2nd page.

And yes, the context menu could even have
* Refresh => searches the same again (no search dialog)
* Modify/Update search (or better name)  => opens the search dialog, and then replace the tab.
Title: Re: Search again
Post by: Paolo on October 17, 2020, 04:43:03 pm
Quote
It only compares the SearchText, not the other search criteria. A user would want to search from different directories, using case sensitive/insensitive, regular expression etc., and have their results in separate tabs

this is partially true, my code leaves separate tabs for example if you enabled or disable "case sensitive" option (see pdf figure (1)), but you are right, it does not recognize some other changed criteria like different path search, thanks to point out this.

here an example

Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. begin
  3.   a:=1;
  4.   A:=2;
  5. end;

and annex in the images of 3 tab results

I was a focused on the "search" and not on the "search and compare", could have sense to have a checkbox like "show search result on separate tab", that when enabled the search works as now, when disable only latest search with same search criteria are shown ?

Quote
The "Search again" in Paolo's popup menu is good. Even better would be to have it in the tab itself, in addition to the TreeView under the tab.
this part of my code seems working correctly (I'll post the code asap), but according to your objection the result of "search again" should be on a separate tab or override the current one ? my view it was an override (essentially it was a refresh after changes in the code, if any), but according to your suggestions it should be shown on another tab for possible comparison, right ?

thanks for any comments.
Title: Re: Search again
Post by: Paolo on October 17, 2020, 05:02:07 pm
What about if we put both (see image)

refesh update (the current tab is override by new results)
search again (results shown on separate tab)

 :D

Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 05:21:48 pm
I was a focused on the "search" and not on the "search and compare", could have sense to have a checkbox like "show search result on separate tab", that when enabled the search works as now, when disable only latest search with same search criteria are shown ?

See my previous comments, yes such a checkbox would help. 

I will see and get some feedback from the rest of the team, as to were it should/could go.


Quote
but according to your objection the result of "search again" should be on a separate tab or override the current one ? my view it was an override (essentially it was a refresh after changes in the code, if any), but according to your suggestions it should be shown on another tab for possible comparison, right ?

"search again" (maybe better named "refresh"/"refresh search" or "update search result"  should replace the existing *tab*.
That is: The "tab" from which it was started.
Therefore:
- IF "update search result" searches with the given setting (without showing a dialoge to change those settings), then the "tab from which it started" is the tab with the same settings.
  BUT: there may be multiple tabs with those exact same settings (if I intentionally did several such searches before).
  So the tab must be stored when the user selects the menu-item (or button).
- IF "update search result" does show a dialoge, the user could change search settings => but it still replaces this tab. (because it is an "update")

Whereas a menu item "new search" could pre-fill the dialoge with the setting from the selected tab => but would not replace the tab.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 05:49:25 pm
What about if we put both (see image)
refesh update (the current tab is override by new results)
search again (results shown on separate tab)
Yes, Martin already suggested it. Then search again would open the search dialog with the same settings.

I was a focused on the "search" and not on the "search and compare", could have sense to have a checkbox like "show search result on separate tab", that when enabled the search works as now, when disable only latest search with same search criteria are shown ?
See my previous comments, yes such a checkbox would help. 
I will see and get some feedback from the rest of the team, as to were it should/could go.
I am against that. A local popup menu on a search result tab will affect the selected tab. It is super intuitive.
If the user wants a new search result tab, he does Ctrl-Shift-F. Again intuitive.
I don't know how you would improve the GUI by adding CheckBoxes around the IDE. There are too many of them already.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 05:54:11 pm
Quote
It only compares the SearchText, not the other search criteria. A user would want to search from different directories, using case sensitive/insensitive, regular expression etc., and have their results in separate tabs
this is partially true, my code leaves separate tabs for example if you enabled or disable "case sensitive" option (see pdf figure (1)), but you are right, it does not recognize some other changed criteria like different path search, thanks to point out this.
Your function IfAlreadySearched in ide.patch does :
Code: Pascal  [Select][+][-]
  1.   if SearchObj.fSearchString = SearchText then begin
  2.     result:=(SearchObj.fSearchOptions = FindInFilesDialogSingleton.Options);
  3.   end;
Clearly it does not compare the "Case sensitive" option. Maybe you have a private patch that does it, but you forgot to upload it.

[Edit]
I realized the search results window already has a speedbutton (up/left) to start a new search. It then copies the search text to the "Find in Files" window. It works partially like the planned Search again feature, but it should copy all criteria in addition to the text.
The next button is Close current page. So, these buttons already work on the selected tab/page. We should add there a Refresh button to make it consistent and complete.
The Refresh and Search again should be in the popup menus as well. More choice is good.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 06:24:16 pm
Better and more intuitive would be to support reordering the tabs by user. Is the current PageControl (or something?) easy to replace with the extended notebook? You have best knowledge of it.
Its a drop in replacement. Should be possible to use the object-inspectors "change class" feature.
It would then need to handle the OnTabDragDrop event (and maybe other OnTab.... events) to make sure the treeviews are correctly associated (not sure how much work is needed there).
I replaced it in r64038.
Wow, it was maybe the easiest change in Lazarus code so far. I only set properties TabDragMode and TabDragAcceptMode to dmAutomatic and it works. No event handling was needed.
Please test.
Title: Re: Search again
Post by: Paolo on October 17, 2020, 07:48:48 pm
Quote
Clearly it does not compare the "Case sensitive" option. Maybe you have a private patch that does it, but you forgot to upload it.

it does in the options comparison !
Code: Pascal  [Select][+][-]
  1. result:=(SearchObj.fSearchOptions = FindInFilesDialogSingleton.Options);
missed something?

a note : looking at the search results of some test cases I see (see code below and image) the "count" in the tab is (1), but it should be (2) [searched word is "aA" with no whole words option active]  is it a bug ? (LAZARUS 2.0.10, FPC 3.2.0)

Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. var
  3.   aA, aaA : integer;
  4. begin
  5.  
  6. end;
  7.  
Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 07:56:26 pm
I think it counts the lines.
But technically that is wrong, it should count matches. So that would be a bug IMHO.

However, if changed, it should NOT count overlapping matches.
Search (case insensitive) "abab" in the below.
Code: Pascal  [Select][+][-]
  1. abABabAb
Matches start at pos: 1, 3 (overlap), 5. But I think it should report 2. But I am open for other opinions.
Title: Re: Search again
Post by: Martin_fr on October 17, 2020, 07:57:00 pm
Wow, it was maybe the easiest change in Lazarus code so far. I only set properties TabDragMode and TabDragAcceptMode to dmAutomatic and it works. No event handling was needed.
Please test.
With ctrl key, it shows "copy" cursor.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 09:39:00 pm
it does in the options comparison !
Code: Pascal  [Select][+][-]
  1. result:=(SearchObj.fSearchOptions = FindInFilesDialogSingleton.Options);
missed something?
Yes, sorry! The Result was set there based on comparison. My bad.

I took the liberty to add a Refresh button there in r64042. It only shows a ToDo item in ShowMessage now. Please feel free to implement.
I still feel the same functionality should be replicating in a local popup menu. Many people prefer textual information over icons.
Title: Re: Search again
Post by: JuhaManninen on October 17, 2020, 11:39:00 pm
With ctrl key, it shows "copy" cursor.
That is a small imperfection. For consistency it should copy the tab if Ctrl-Drag is the standard way to do it.
TinyPortal © 2005-2018