Skip to content

Replace TextMatcher in favor of extended StringMatcher #2672

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 8, 2025
Merged

Replace TextMatcher in favor of extended StringMatcher #2672

merged 1 commit into from
Jan 8, 2025

Conversation

@ptziegler
Copy link
Contributor

@ptziegler ptziegler commented Jan 7, 2025

The functionality of the TextMatcher has been integrated in the StringMatcher class from the Equinox bundle, which allows this class to be used without a dependency to the Eclipse UI bundle.

Note: To get the same behavior as the TextMatcher, one needs to call matchWords() instead of match(). Furthermore, the pattern needs to be trimmed explicitly, where needed.

Contributes to
#2567

@ptziegler
Replace TextMatcher in favor of extended StringMatcher
ce5e412 
The functionality of the TextMatcher has been integrated in the
StringMatcher class from the Equinox bundle, which allows this class to
be used without a dependency to the Eclipse UI bundle.

Note: To get the same behavior as the TextMatcher, one needs to call
matchWords() instead of match(). Furthermore, the pattern needs to be
trimmed explicitly, where needed.

Contributes to
eclipse-platform#2567
@iloveeclipse
Copy link
Member

iloveeclipse commented Jan 7, 2025

The functionality of the TextMatcher has been integrated in the StringMatcher class from the Equinox bundle

Please update bundle requirements to expect minimal version of equinox containing the new API

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Test Results

 1 818 files   -  3   1 818 suites   - 3   1h 37m 56s ⏱️ + 6m 51s
 7 732 tests  -  6   7 504 ✅  -  5  228 💤 ±0  0 ❌  - 1 
24 357 runs   - 18  23 608 ✅  - 17  749 💤 ±0  0 ❌  - 1 

Results for commit ce5e412. ± Comparison against base commit 58bb65c.

This pull request removes 6 tests. 
UiTestSuite org.eclipse.ui.tests.filteredtree.TextMatcherTest ‑ testCaseInsensitivity
UiTestSuite org.eclipse.ui.tests.filteredtree.TextMatcherTest ‑ testChinese
UiTestSuite org.eclipse.ui.tests.filteredtree.TextMatcherTest ‑ testEmpty
UiTestSuite org.eclipse.ui.tests.filteredtree.TextMatcherTest ‑ testMultipleWords
UiTestSuite org.eclipse.ui.tests.filteredtree.TextMatcherTest ‑ testSingleWords
UiTestSuite org.eclipse.ui.tests.filteredtree.TextMatcherTest ‑ testSuffixes

@ptziegler
Copy link
Contributor Author

ptziegler commented Jan 7, 2025

Please update bundle requirements to expect minimal version of equinox containing the new API

The Equinox bundle is re-exported by org.eclipse.core.runtime. So I assume you mean to adjust the version of the required bundle there and then bump the dependency to the runtime here?

@iloveeclipse
Copy link
Member

iloveeclipse commented Jan 7, 2025

The Equinox bundle is re-exported by org.eclipse.core.runtime. So I assume you mean to adjust the version of the required bundle there and then bump the dependency to the runtime here?

If core.runtime reexports new equinox API amd in this release didn't increased minor version segment, it has to bump it and after that the ui bundles that consume new API should update minimal range of core.runtime.

ptziegler added a commit to ptziegler/eclipse.platform that referenced this pull request Jan 7, 2025
@ptziegler
Increase lower bound to re-exported Equinox Common
d7fed65 
New API has been added to the StringMatcher, which is going to be used
in the Platform UI bundles.

See eclipse-platform/eclipse.platform.ui#2672
@ptziegler ptziegler mentioned this pull request Jan 7, 2025
Merged
iloveeclipse pushed a commit to eclipse-platform/eclipse.platform that referenced this pull request Jan 7, 2025
@ptziegler @iloveeclipse
Increase lower bound to re-exported Equinox Common
1a3936f 
New API has been added to the StringMatcher, which is going to be used
in the Platform UI bundles.

See eclipse-platform/eclipse.platform.ui#2672
@vogella vogella merged commit db41fb4 into eclipse-platform:master Jan 8, 2025
17 checks passed
@ptziegler
Copy link
Contributor Author

ptziegler commented Jan 8, 2025

I still haven't updated the lower bound for the dependency to org.eclipse.core.runtime, though...

@vogella
Copy link
Contributor

vogella commented Jan 8, 2025

I still haven't updated the lower bound for the dependency to org.eclipse.core.runtime, though...

Please push another PR for this.

@iloveeclipse
Copy link
Member

iloveeclipse commented Jan 8, 2025

@vogella : please next time before merge read comments on PR.
@ptziegler : please add requested changes, thanks.

@ptziegler
Copy link
Contributor Author

ptziegler commented Jan 8, 2025

#2673

@ptziegler ptziegler deleted the replace-text-matcher-with-string-matcher branch January 8, 2025 05:36
@iloveeclipse iloveeclipse mentioned this pull request Jan 8, 2025
Merged
@vogella
Copy link
Contributor

vogella commented Jan 8, 2025

eclipse-equinox/p2#604

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ptziegler @iloveeclipse @vogella