Quick search fix #2541

jannisCode · 2024-11-26T12:51:09Z

This PR adresses the issue posed here: #2329

eclipse-platform-bot · 2024-11-26T12:56:43Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 4c83dfa0c04fa6dd30844f7bab1bbbb9a020679a Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Tue, 17 Dec 2024 09:19:07 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF b/bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF
index d656db96ed..dae450f163 100644
--- a/bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.text.quicksearch;singleton:=true
-Bundle-Version: 1.2.500.qualifier
+Bundle-Version: 1.2.600.qualifier
 Bundle-Activator: org.eclipse.text.quicksearch.internal.ui.QuickSearchActivator
 Require-Bundle: org.eclipse.ui;bundle-version="[3.113.0,4.0.0)",
  org.eclipse.core.resources;bundle-version="[3.13.0,4.0.0)",
-- 
2.47.1

Further information are available in Common Build Issues - Missing version increments.

github-actions · 2024-11-26T13:06:50Z

Test Results

1 214 files ±0 1 214 suites ±0 1h 14m 46s ⏱️ -55s
7 729 tests ±0 7 496 ✅ +1 233 💤 ±0 0 ❌ - 1
16 232 runs ±0 15 717 ✅ +1 515 💤 ±0 0 ❌ - 1

Results for commit c71a9ed. ± Comparison against base commit 0344b2b.

♻️ This comment has been updated with latest results.

HannesWell

Thank you for this PR.
I'm not very familiar with this part of the code and therefore just did a 'code-style' review. Please see my remarks below.

@Wittmaxi or @HeikoKlare do you plan to do a 'functional' review? If not I can have a look too.

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java

...eclipse.text.quicksearch/src/org/eclipse/text/quicksearch/internal/ui/QuickSearchDialog.java

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java

fedejeanne

I wouldn't add the empty element to the history. It should be active if it was the last one in use but I wouldn't put it in the list since one can always easily recreate the "empty" by deleting the content:

I also have some comment regarding the names of the new field and constant in DialogSettings and the spacing.

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java

...eclipse.text.quicksearch/src/org/eclipse/text/quicksearch/internal/ui/QuickSearchDialog.java

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java

fedejeanne

I tested it and it works as expected. I took the liberty of doing the following changes though, I thought it would be faster than reviewing again:

Removed unused constant
Inlined method getLastSearches()
Removed some unnecessary variables in methods
Made the field filterHistory final and non-static
Stored/retrieved the lasat searches (filterHistory) from the settings to allow the user to reuse the history even across restarts
Other small cosmetic changes (moved lines around)

I will merge this PR as soon as the tests are finished. @jannisCode please don't do any more changes.

fedejeanne · 2024-12-17T14:28:31Z

@HannesWell may I drop your request for changes?

fedejeanne · 2024-12-17T15:31:41Z

The test failure on Mac is unrelated: #294

The other 2 failures are confusing though, they are very similar but one fails for the plugin org.eclipse.search and the other one (Windows) fails for org.eclipse.e4.ui.swt.gtk

Linux

Error:  Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.10:verify (verify) on project org.eclipse.search: Execute ApiApplication failed: InvocationTargetException: Component 'Search Support' in the baseline 'workspace' is disposed [main]: Component was disposed here [Worker-1: Updating plug-in dependencies] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.10:verify (verify) on project org.eclipse.search: Execute ApiApplication failed
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:333)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
...

Windows

Error:  Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.10:verify (verify) on project org.eclipse.e4.ui.swt.gtk: Execute ApiApplication failed: InvocationTargetException: Component '%fragmentName' in the baseline 'workspace' is disposed [main]: Component was disposed here [Worker-1: Updating plug-in dependencies] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.10:verify (verify) on project org.eclipse.e4.ui.swt.gtk: Execute ApiApplication failed
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:333)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
...

I'm triggering the builds again.

Fixes eclipse-platform#2329 Co-authored-by: Federico Jeanne <[email protected]>

laeubi · 2024-12-17T15:35:21Z

@fedejeanne see

Error logged from API Tools Core: Component in baseline is disposed eclipse-pde/eclipse.pde#145

Requests have been addressed.

HannesWell

@HannesWell may I drop your request for changes?

Yes, I dropped it. Thanks for the heads up.
I just a have another suggestion, but not crucial.

HannesWell · 2024-12-17T22:08:17Z

...eclipse.text.quicksearch/src/org/eclipse/text/quicksearch/internal/ui/QuickSearchDialog.java

+				String filter = filterHistory.getFirst();
+
+				// Filter out blanks
+				filterHistory.removeIf(String::isBlank);
+
+				searchIn.setItems(filterHistory.toArray(String[]::new));


Nitpick: I don't think we need a comment to explain the next line

Suggested change

String filter = filterHistory.getFirst();

// Filter out blanks

filterHistory.removeIf(String::isBlank);

searchIn.setItems(filterHistory.toArray(String[]::new));

String filter = filterHistory.getFirst();

filterHistory.removeIf(String::isBlank);

searchIn.setItems(filterHistory.toArray(String[]::new));

I'd normally agree but seeing that the builds are so brittle, I'd like to leave it as it is.

jukzi · 2025-01-15T14:52:06Z

This PR caused a NPE on first open:
#2706

jannisCode force-pushed the Quick_search_fix branch from b79d8df to 811b39d Compare November 26, 2024 13:14

jannisCode mentioned this pull request Nov 26, 2024

Add dropdown history to the "in" field of the Quick Search dialog window #2329

Closed

jannisCode force-pushed the Quick_search_fix branch 3 times, most recently from bb06744 to cfebba0 Compare November 26, 2024 13:17

mickaelistria force-pushed the Quick_search_fix branch from b45939b to bd6c3f7 Compare November 26, 2024 17:21

jannisCode force-pushed the Quick_search_fix branch 2 times, most recently from ec72ad4 to 0b07e25 Compare November 28, 2024 17:59

jannisCode marked this pull request as ready for review November 29, 2024 09:24

jannisCode force-pushed the Quick_search_fix branch 3 times, most recently from 47d5efe to 1fd490f Compare December 6, 2024 13:04

HannesWell previously requested changes Dec 6, 2024

View reviewed changes

jannisCode force-pushed the Quick_search_fix branch 2 times, most recently from ac14b93 to 8ea099c Compare December 9, 2024 08:43

fedejeanne requested changes Dec 10, 2024

View reviewed changes

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java Outdated Show resolved Hide resolved

jannisCode force-pushed the Quick_search_fix branch 2 times, most recently from c9772f2 to 77f5171 Compare December 10, 2024 12:22

fedejeanne requested changes Dec 10, 2024

View reviewed changes

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java Outdated Show resolved Hide resolved

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java Outdated Show resolved Hide resolved

jannisCode force-pushed the Quick_search_fix branch from fa0b3bb to 0a0a010 Compare December 10, 2024 14:16

fedejeanne requested changes Dec 16, 2024

View reviewed changes

...eclipse.text.quicksearch/src/org/eclipse/text/quicksearch/internal/ui/QuickSearchDialog.java Outdated Show resolved Hide resolved

bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/DialogSettings.java Outdated Show resolved Hide resolved

jannisCode force-pushed the Quick_search_fix branch 2 times, most recently from bb59c93 to 50bc99d Compare December 17, 2024 09:14

fedejeanne force-pushed the Quick_search_fix branch 2 times, most recently from 0cfa7c9 to 104d3e3 Compare December 17, 2024 14:23

fedejeanne approved these changes Dec 17, 2024

View reviewed changes

fedejeanne linked an issue Dec 17, 2024 that may be closed by this pull request

Add dropdown history to the "in" field of the Quick Search dialog window #2329

Closed

jannisCode and others added 2 commits December 17, 2024 16:34

Drop Down menu in Quick Search implemented

bb8778f

Fixes eclipse-platform#2329 Co-authored-by: Federico Jeanne <[email protected]>

Version bump(s) for 4.35 stream

c71a9ed

fedejeanne force-pushed the Quick_search_fix branch from 104d3e3 to c71a9ed Compare December 17, 2024 15:34

HannesWell reviewed Dec 17, 2024

View reviewed changes

fedejeanne merged commit 3325875 into eclipse-platform:master Dec 18, 2024
16 of 17 checks passed

RedeemerSK mentioned this pull request Dec 21, 2024

Display quick search dialog result in SourceViewer with line numbers. Fixes #2010 #2644

Merged

3 tasks

Quick search fix #2541

Quick search fix #2541

Uh oh!

Conversation

jannisCode commented Nov 26, 2024

Uh oh!

eclipse-platform-bot commented Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

HannesWell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

fedejeanne commented Dec 17, 2024

Uh oh!

fedejeanne commented Dec 17, 2024

Linux

Windows

Uh oh!

laeubi commented Dec 17, 2024

Uh oh!

HannesWell left a comment

Choose a reason for hiding this comment

Uh oh!

HannesWell Dec 17, 2024

Choose a reason for hiding this comment

Uh oh!

fedejeanne Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jukzi commented Jan 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eclipse-platform-bot commented Nov 26, 2024 •

edited

Loading

github-actions bot commented Nov 26, 2024 •

edited

Loading