Community
Participate
Working Groups
AIM: search for a text (say, 'supplierOID') in various source code files Means: use the search toolbar button ( torch ) action: click on the torch icon input the value 'supplierOID' select the right tab , in our case the text Problem:
Problem : the most common action is to search for text in files under some directory ( like grep -i abc * ) but the default focus comes to 'Java Search'. There should be a way to set the default tab to 'File Search'.
>the most common action is to search for text in files under some directory might be true for you but not for others ;-) >There should be a way to set the default tab to 'File Search'. The Search dialog no longer tries to be smart but opens with the last used Search i.e. if you search for text once then it will come up with this page the next time you open the Search dialog.
fine with me if that was the intended design. Thx anyway
We had lots of opinions about this and changed it several times. Which build are you using? Wouldn't the 2.1 behavior be what you want (instead of setting your default search page via preference you simply have to do one search to get this).
Sorry - I was wrong. We reverted the old behavior back. I reopen the PR to consider for 2.2. What you would like is a preference where you can specify the default page - correct?
exactly. Like so many other things in eclipse a developer should be able to set a default preference on the 'favourite' search tab too. Hope it makes it in the 2.2 ( btw i am on 2.1)
*** Bug 34680 has been marked as a duplicate of this bug. ***
see also bug 34680
*** Bug 34799 has been marked as a duplicate of this bug. ***
*** Bug 48103 has been marked as a duplicate of this bug. ***
We should do something for 3.0. Any idea why we never went to the (simpler than now) solution to just remember the last page?
- we wanted to be smarter than just remember the last one - the last used page might no longer exist
I think we should add the "last" page policy as an option. Marking as 3.0 candidate.
Not critical and too late now for 3.0
Note that users can always assign another shortcut to 'Search > File Search' via the 'Workbench > Keys' preference page. If you want, you can even assign Ctrl+H to File Search.
*** Bug 237160 has been marked as a duplicate of this bug. ***
*** Bug 279741 has been marked as a duplicate of this bug. ***
No plans to fix this but we should add a way to allow reopening the dialog with the previous settings, see see bug 199990.
*** Bug 331617 has been marked as a duplicate of this bug. ***
*** Bug 351583 has been marked as a duplicate of this bug. ***
Reopening as Marko offered to provide a patch.
(In reply to comment #22) > Reopening as Marko offered to provide a patch. The place where the recommended editor is opened is in method SearchDialog.getPreferredPageIndex()
Created attachment 199804 [details] Patch for plugin "org.eclipse.search"
I will provide a patch for the ...doc.user plugin too as soon as I can figure out how to create a patch of a changed png picture. Does anybody know how to do that using the git command line?
Marko, Search and Doc are currently live in CVS not Git. We're planning to move this summer. Could you attach a CVS workspace patch? Images need to be attached separately (best in a ZIP file).
(In reply to comment #26) > Marko, Search and Doc are currently live in CVS not Git. We're planning to move > this summer. Could you attach a CVS workspace patch? > > Images need to be attached separately (best in a ZIP file). :( Our company firewall is completely blocking any CVS access so I have to make it at home. This will take some time ( a few days).
Created attachment 199965 [details] Workspace patch (GIT format) This is a GIT created patch for my changes. I hope that applying it is ok for you. Otherwise I need to do all changes at home via the CVS access. The new picture will be added seperately.
Created attachment 199966 [details] Updated picture for doc.user plugin
I'll try when I find time (currently completely busy with Java 7).
Will review during M5.
Hi Marko, I can apply the patch but it does not work i.e. it does not use the last used search page. Looking at the patch it's obvious: there is no code that stores the last used page ;-). Some other comments: - We already have a 'Customize...' button (and dialog) inside the search dialog. This should be used to host the new option. - The last used page should not be stored as a preference but as dialog setting. - Test corner cases of your next patch: - plug-in that contributes last used page gets removed - last used search page gets disabled via 'Customize...' - I don't like "Focus" in the option label (and the doc). How about "Show (or Activate) last used search page" "Show (or Activate) best fitting search page" - The Javadoc of 'ISearchPageScoreComputer' and the 'extensions' attribute in the 'org.eclipse.search.searchPages' extension point doc also needs to be updated. - Use the formatting already present in the files, e.g. we use compact assignment. - Add your credentials to each file, e.g. Dani Megert <dani@eclipse.org> - this is a bug - http://bugs.eclipse.org/33710 - Update copyright date to 2012 in each file.
Marko, are you still interested in providing the patch?
I will not have time for the patch for the next few weeks. Need the time for renovating my house :) Feel free to do the patch on your own.
(In reply to comment #34) > I will not have time for the patch for the next few weeks. Need the time for > renovating my house :) > > Feel free to do the patch on your own. I don't have time for this.
*** Bug 395453 has been marked as a duplicate of this bug. ***
*** Bug 397385 has been marked as a duplicate of this bug. ***
I am currently working on a follow-up patch so please consider my comments/questions: > - We already have a 'Customize...' button (and dialog) inside the search > dialog. This should be used to host the new option. The Customize... is currently implemented as ListSelectionDialog, adding an option like "Remember selected search tab" e.g. as Check Button at the end, requires substantial modification to this Dialog. Is this definitely the way to go? What about simply adding the Check Button "Remember selected search tab" directly to the Search Dialog? >- The last used page should not be stored as a preference but as dialog setting. Alright. #turnToPage() SearchPlugin.getDefault().getDialogSettings().put(PREVIOUS_SEARCH_PAGE, descriptor.getId()); >- Test corner cases of your next patch: > - plug-in that contributes last used page gets removed > - last used search page gets disabled via 'Customize...' Will consider this eventually >- I don't like "Focus" in the option label (and the doc). How about > "Show (or Activate) last used search page" > "Show (or Activate) best fitting search page" Will not exist if option is to be embedded in "Customize..." >- The Javadoc of 'ISearchPageScoreComputer' and the 'extensions' attribute in > the 'org.eclipse.search.searchPages' extension point doc also needs to be > updated. Alright
Created attachment 225249 [details] Proposed solution for the bug I implemented a new patch for this problem, considering the given comments. >- We already have a 'Customize...' button (and dialog) inside the search > dialog. This should be used to host the new option. Done. Screenshot to follow >- The last used page should not be stored as a preference but as dialog setting. Done >- Test corner cases of your next patch: > - plug-in that contributes last used page gets removed Falls back to default search page. (Tried using invalid descriptor string) > - last used search page gets disabled via 'Customize...' Same. >- I don't like "Focus" in the option label (and the doc). How about > "Show (or Activate) last used search page" > "Show (or Activate) best fitting search page" Is now "Remember selected search tab" >- The Javadoc of 'ISearchPageScoreComputer' and the 'extensions' attribute in > the 'org.eclipse.search.searchPages' extension point doc also needs to be > updated. I am not sure what the wanted behavior is, so the current implementation disregards any intelligent selection and sets the last used. If the behaviour should consider a "last selected" per intelligent selection the implementation has to be made entirely different than now. Other points: Currently after disabling the feature the set last tab is not deleted, so if the feature is re-enabled the lastly selected tab is re-opened. DialogSettings does not support the removal of entries, this is already documented in Bug 92518.
Created attachment 225250 [details] New Customize... dialog
> "Remember selected search tab" The problem with this label is that the alternative is not clear. This solution makes clear that the option only affects the 'Search...' command and describes both alternatives: 'Search...' opens: o Last used tab o Best tab for selection
Created attachment 225310 [details] Alternate solution with Radio selector I see, do you think about something like a selector in the customize? The enclosed patch realizes it using a Radio button, certainly there could also be combo etc. I'll finish that out as soon as the "specification" is set :)
Created attachment 225311 [details] Search Customize Dialog with selection The Customize.. dialog with resp. radio selection
Anybody interested in giving a comment to the proposed patch?
Comment on attachment 225249 [details] Proposed solution for the bug This patch does not seem to work and it does an unwanted page switch after confirming the dialog: 1. start new workspace 2. open Search 3. switch to 'Java Search' 4. change the setting ==> page flips back to 'File Search', expected: stays on the current page 5. close the dialog 6. open it again BUG: opens on 'File Search' I don't like the option name. Maybe better: &Remember last used page (The '&' denotes the mnemonic which missing in your patch).
Comment on attachment 225310 [details] Alternate solution with Radio selector This patch works better than the previous one, but using radio buttons for this on/off option is not nice.
The "Remember last used page" overrides the selection in any case, so the user should know about that behaviour, shouldn't she? Currently there is some kind of intelligent selection depending on the editor opened, this would be completely overriden, hence the intent to provide a radio button to explicitely show this to the user. A on/off option only would not transfer the meaning of the option correctly, as one may think that it sets the "remember last page" per intelligent selection one had. E.g. I called search from within a Java editor, and now everytime I call it from a Java editor leads to the last selected search page I had in this context.
(In reply to comment #47) > The "Remember last used page" overrides the selection in any case, so the > user should know about that behaviour, shouldn't she? Sorry, I'm not sure what you try to say/explain here. > Currently there is some kind of intelligent selection depending on the > editor opened, this would be completely overriden, hence the intent to > provide a radio button to explicitely show this to the user. Having "Remember last used page" checked explains that too.
Created attachment 227716 [details] Solution with Checkbox (and formatter) Please have a look at the enclosed patch which solves the problem using Check. I have been trying to get the formatting right to remove all the noise. What formatting style is used in this class? I am using the current one but this does not work out. It should, however, be possible to grasp the main changes quite fast.
Comment on attachment 227716 [details] Solution with Checkbox (and formatter) The functionality is good now and works as expected! In order to review (and later commit), the patch I need one that has only changes where needed and which does not reformat existing untouched code. Please also update the copyright notice (and date) in all the files that you touch. > I have been trying to get the formatting right to remove all the noise. Actually, doing no formatting at all on the existing code and just format the code that you add (e.g. using a Save Action that only formats the changed lines). > What formatting style is used in this class? Normally, we use the profile that's attached to the project, but we never re-formatted all the code at the time when we settled on that profile.
Created attachment 227721 [details] Formatting corrected version There you go. Enclosed, the corrected version according to your rules.
Comment on attachment 227721 [details] Formatting corrected version Thanks Marco. I reviewed the patch and it is good except for some minor details: - defaultDialogSettings has several issues: - field names are prefixed with 'f' - we must not write into the root of that store, but create/use a section, see DialogSettings.getOrCreateSection(IDialogSettings, String) - improve the dialog setting constants (see TextSearchPage for an example) - use speaking names for variables (e.g. 'b' is not so good) - replace nested if with one if and a && - rename the NLS key to SearchPageSelectionDialog_rememberLastUsedPage_message After that polish pass, the patch is good to go!
Created attachment 227800 [details] Reworked patch according to Hy Dani, thanks for your feedback, enclosed the reworked patch according to your comments.
(In reply to comment #53) > Created attachment 227800 [details] [diff] > Reworked patch according to > > Hy Dani, thanks for your feedback, enclosed the reworked patch according to > your comments. I've committed your patch with http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=b93444c2bf7a4607e4741b8aa0a51ce82d28a357 After that I made some minor tweaks: - used better names for 'ret' and 'fDefaultDialogSettings' - fixed some wrong indentations - also store the last used page if the user never manually switched to another page - added bug summary to copyright (forgot to mention this in the previous review) Fixed with http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=97b1cf4b6990936dca33ed932af778c661f5fd04
The setting must only be saved when user closes the Customize dialog with OK. Fixed with http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=8eb5d915d6bdb101cbb3597425c9892ee08f3fab
Verified in I20130312-2000.
