Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 31, 2025

Conversation

RedeemerSK
Copy link
Contributor

@RedeemerSK RedeemerSK commented Dec 21, 2024

What it does

Adds line numbers (vertical ruler) when presenting content that contains searched term in Quick Search dialog. For this, simple StyledText widget that was used to present content was replaced with SourceViewer widget. It's configured with same colors like generic text editor.

  • also adds standard current line highlighting based on caret position (*)
  • also always highlights (with 'current line highlight' background color) the line that contains selected match regardless of caret position - to help telling apart target match / line amongst multiple matches on other lines possibly present around target match (*)
  • also scrolls horizontally, if necessary, to reveal 1st match of the searched term in the target line
  • reacts to individual color preferences changes even if opened

(* current line highlighting must be enabled in preferences)

How to test

Open Quick Search dialog (default: CTRL + SHIFT + ALT + L) and confirm bottom area presenting parts of files where match was found behaves like it used to - displays match vertically centered, keeps it centered on resizing the dialog or area, displays match 'context' (lines before & after match within source file).

Known bugs

Changing theme (light <-> dark) while Quick Search dialog is open does not change all the colors used inside it (namely occurrences highlighting). Closing and reopening the dialog fixes the colors.

Author checklist

@RedeemerSK
Copy link
Contributor Author

RedeemerSK commented Dec 21, 2024

(following pictures were taken before #2541 (adding 'search in' combo box with history) was merged)

Before
before_light
before_dark
After
after_light
after_dark

After + caret located on line other than target line, caret line becomes highlighted as well
after_dark_line_highlight

@BeckerWdf
Copy link
Contributor

Thats a nice addition.

@@ -1030,47 +1168,6 @@ private StyledString highlightMatches(String visibleText) {
return styledText;
}

// Version using sourceviewer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we know why this was discarded in the first place?

Copy link
Contributor Author

@RedeemerSK RedeemerSK Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was already there (see 1st commit) when QuickSearchDialog code was contributed from Spring Tools suite.
From what I can tell it was always just a protoype code - when I tried it uncommented it looked just like that.

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Test Results

 1 824 files  ±0   1 824 suites  ±0   1h 37m 22s ⏱️ -6s
 7 918 tests ±0   7 690 ✅ ±0  228 💤 ±0  0 ❌ ±0 
23 841 runs  ±0  23 093 ✅ ±0  748 💤 ±0  0 ❌ ±0 

Results for commit f636a25. ± Comparison against base commit 9cd93e5.

♻️ This comment has been updated with latest results.

@RedeemerSK
Copy link
Contributor Author

Superseded by #2853

@BeckerWdf would you mind looking into the new PR as well ? Thanks

@mickaelistria
Copy link
Contributor

mickaelistria commented Mar 31, 2025

I'd like to see this merged ASAP. Code looks good enough, and all the change being internal makes risk very low.
Currently, the build is failing with Failed to execute goal org.eclipse.tycho.extras:tycho-p2-extras-plugin:4.0.13-SNAPSHOT:compare-version-with-baselines (compare-attached-artifacts-with-release) on project org.eclipse.text.quicksearch: Only qualifier changed for (org.eclipse.text.quicksearch/1.3.0.v20250330-2041). Expected to have bigger x.y.z than what is available in baseline (1.3.0.v20250123-0754) . @RedeemerSK Can you please append a commit that bumps the version to 1.3.100.qualifier (since all changes are internal/private) ?
@BeckerWdf Is there anything you think is still worth trying to improve before merging?

@eclipse-platform-bot
Copy link
Contributor

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 e9461d385dddb37b02e2ebe2660a779dd2483942 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Mon, 31 Mar 2025 09:26:14 +0000
Subject: [PATCH] Version bump(s) for 4.36 stream


diff --git a/bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF b/bundles/org.eclipse.text.quicksearch/META-INF/MANIFEST.MF
index f158e0beb1..426765fded 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.3.0.qualifier
+Bundle-Version: 1.3.100.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.49.0

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

@mickaelistria
Copy link
Contributor

@RedeemerSK It looks like you haven't signed the Eclipse Contributor Agreement. Could you please review and sign it at https://accounts.eclipse.org/user/eca ?

@iloveeclipse
Copy link
Member

@RedeemerSK It looks like you haven't signed the Eclipse Contributor Agreement. Could you please review and sign it at https://accounts.eclipse.org/user/eca ?

I believe it's foundation issue, I'm also not recognized anymore to have ECA in #2873 :-)

@iloveeclipse
Copy link
Member

See https://www.eclipsestatus.io
image

@mickaelistria mickaelistria merged commit fe36341 into eclipse-platform:master Mar 31, 2025
18 checks passed
@mickaelistria
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants