Recent

Author Topic: Search again  (Read 7640 times)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Search again
« Reply #15 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)


Paolo

  • Sr. Member
  • ****
  • Posts: 499
Re: Search again
« Reply #16 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.





Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Search again
« Reply #17 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.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: Search again
« Reply #18 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.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Search again
« Reply #19 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

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: Search again
« Reply #20 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.
« Last Edit: October 08, 2020, 08:29:03 pm by JuhaManninen »
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

Paolo

  • Sr. Member
  • ****
  • Posts: 499
Re: Search again
« Reply #21 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 ?

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Search again
« Reply #22 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.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: Search again
« Reply #23 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. :)
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Search again
« Reply #24 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).

Paolo

  • Sr. Member
  • ****
  • Posts: 499
Re: Search again
« Reply #25 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.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: Search again
« Reply #26 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.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

Paolo

  • Sr. Member
  • ****
  • Posts: 499
Re: Search again
« Reply #27 on: October 17, 2020, 03:21:23 pm »
attached file with some typos corrected

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Search again
« Reply #28 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?

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: Search again
« Reply #29 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.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

 

TinyPortal © 2005-2018