possibility to suppress GTK interactive search (popup) #1829

chrstg · 2025-02-14T13:16:57Z

No description provided.

iloveeclipse

Please provide meaningful commit message.

See

After that, please amend your commit and force push to same branch.

Please note, that we are at 4.35 RC phase so no API changes are allowed for this release.

github-actions · 2025-02-14T13:29:40Z

Test Results

541 files ±0 541 suites ±0 30m 11s ⏱️ + 2m 44s
4 364 tests ±0 4 349 ✅ ±0 15 💤 ±0 0 ❌ ±0
16 596 runs ±0 16 486 ✅ ±0 110 💤 ±0 0 ❌ ±0

Results for commit f213a14. ± Comparison against base commit b29048a.

♻️ This comment has been updated with latest results.

bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/SWT.java

chrstg · 2025-02-17T07:19:11Z

Please note, that we are at 4.35 RC phase so no API changes are allowed for this release.

So we'll have to wait for the next release.
Anything to do for me until then?

iloveeclipse · 2025-02-17T07:22:26Z

Anything to do for me until then?

Yes. Currently you have 3 commits including one merge commit.
Merge commits are not allowed, and for such a small change there is no need for multiple commits either.

Please squash your changes to one commit, rebase it on top of master branch and force push to same branch on github.

chrstg · 2025-02-17T07:24:36Z

Anything to do for me until then?

Yes. Currently you have 3 commits including one merge commit. Merge commits are not allowed, and for such a small change there is no need for multiple commits either.

Please squash your changes to one commit, rebase it on top of master branch and force push to same branch on github.

isn't it good practive to separate functional commit and version increase commit?

iloveeclipse · 2025-02-17T07:33:47Z

isn't it good practive to separate functional commit and version increase commit?

Correct, however I can't distinguish your commits in github UI because they have exact same commit message.
So updated instructions were: get rid of merge commit, provide proper commit messages, rebase on master & force push.

chrstg · 2025-02-17T09:36:21Z

isn't it good practive to separate functional commit and version increase commit?

Correct, however I can't distinguish your commits in github UI because they have exact same commit message. So updated instructions were: get rid of merge commit, provide proper commit messages, rebase on master & force push.

force push is rejected with message
! [remote rejected] master -> master (refusing to allow a Personal Access Token to create or update workflow .github/workflows/maven.yml without workflow scope)

Either way i didn't branch on my fork.
I guess it's better to start over from scratch with a new pull request?!

iloveeclipse · 2025-02-17T09:58:55Z

I guess it's better to start over from scratch with a new pull request?!

Yes. Id you work on master, you shouldn't do this in the future.
See also https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#creating-a-pull-request

merks · 2025-02-17T10:19:06Z

What I sometimes do is locally rename the branch I'm using for the PR, check out master again, pull, create a branch with the name previously used for the PR, commit the changes to that branch, and then force push that. That updates the current PR with completely new clean content rather than close a PR and open a new PR...

chrstg · 2025-02-17T10:31:07Z

What I sometimes do is locally rename the branch I'm using for the PR, check out master again, pull, create a branch with the name previously used for the PR, commit the changes to that branch, and then force push that. That updates the current PR with completely new clean content rather than close a PR and open a new PR...

Thanks for the hint!
.. i managed the rebase, but I'm still on my local master branch

chrstg · 2025-02-17T10:32:25Z

the Jenkins build failes with
04:51:13 [ERROR] [API ERROR] File unknown at line 0: The field org.eclipse.swt.SWT.NO_SEARCH has been added to a class (location: unknown)
04:51:13 [ERROR] [API ERROR] File MANIFEST.MF at line 1: The major version should be incremented in version 3.130.0, since API breakage occurred since version 3.128.0 (location: /home/jenkins/agent/workspace/eclipse.platform.swt_PR-1829/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.ppc64le/META-INF/MANIFEST.MF)

Did i increase the versions in a wrong way?

merks · 2025-02-17T12:13:26Z

The API tools are very conservative about what is a breaking change. Adding a public static field to a class is considered API-breaking and hence it wants a 4.0.0. But that's not really acceptable so the error needs to be suppressed. Normally that's done via the a quick-fix in the IDE; was the error not already evident in the IDE?

chrstg · 2025-02-18T08:19:58Z

The API tools are very conservative about what is a breaking change. Adding a public static field to a class is considered API-breaking and hence it wants a 4.0.0. But that's not really acceptable so the error needs to be suppressed. Normally that's done via the a quick-fix in the IDE; was the error not already evident in the IDE?

In first place i didn't set up API tools / baseline correctly. Now it showed up and offered the quick fixes.

chrstg · 2025-02-18T08:22:32Z

btw, the binary fragments require host bundle-version="[3.128.." . couldn't it be increased to [3.129 ?

merks · 2025-02-18T08:24:56Z

Yes, I think these much match. Note that you will need to redo all this at the start of the next release cycle, so I'm not sure you want to bang your head on this now...

merks · 2025-02-18T08:27:55Z

In first place i didn't set up API tools / baseline correctly. Now it showed up and offered the quick fixes.

It's probably better to use the setup which does all the necessary and desired things:

https://github.com/eclipse-platform/eclipse.platform.swt?tab=readme-ov-file#contributing-to-swt

E.g., it will update the target platform and the API baseline when the time comes for that...

chrstg · 2025-02-18T09:15:37Z

Yes, I think these much match. Note that you will need to redo all this at the start of the next release cycle, so I'm not sure you want to bang your head on this now...

i was afraid of that :D but next time it'll go in a minute
parts of the .api_filters can be removed then

chrstg · 2025-03-19T12:04:12Z

Hi @iloveeclipse
can we get this one through the gate.

iloveeclipse · 2025-03-20T10:35:14Z

bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java

 	if ((getShell ().style & SWT.ON_TOP) != 0) {
 		GTK.gtk_tree_view_set_search_column (handle, -1);
 	}
+    if ((style & SWT.NO_SEARCH) != 0) {


Can we combine this "if" with the previous one, in a style consistent with the Tree / Table code below and extract the condition into searchEnabled() method?

sure, did it.
(even when imho i think that the implementation of the no_search() method is ugly, too many negations)

iloveeclipse · 2025-03-20T10:50:26Z

can we get this one through the gate.

See my comment in code above, beside this it looks good & ready to merge. If you change the code, please first rebase on master, then amend commit & force push.

chrstg · 2025-03-21T15:46:36Z

the build fails on PLATFORM = 'gtk.linux.riscv64'
java.nio.file.FileSystemException: /home/jenkins/workspace/eclipse.platform.swt_PR-1829: Read-only file system

i assume it's not from my changes ?!

iloveeclipse · 2025-03-21T15:52:22Z

i assume it's not from my changes ?!

For sure not. It's Friday, both hard- and software is already in the weekend mode :-)
I've restarted the build.

chrst4 · 2025-03-24T06:58:43Z

i assume it's not from my changes ?!

For sure not. It's Friday, both hard- and software is already in the weekend mode :-) I've restarted the build.

something seems broken on the linux-risc build. All pull requests seem to fail there.

Just a test if we can rebuild natives on gtk.linux.riscv64 See eclipse-platform#1829

iloveeclipse · 2025-03-24T09:29:34Z

something seems broken on the linux-risc build. All pull requests seem to fail there.

I've just triggered #1931 to have a proof.

tables eclipse-platform#1830 add SWT.NO_SEARCH flag used in Table, Tree and List to prevent the interactive search (popup) in a GTK environment when set to true.

iloveeclipse · 2025-03-24T10:14:02Z

I've just triggered #1931 to have a proof.

At least that one didn't wanted to rebuild binaries and failed for a different reason, but it was on top of master (yours is not). Let rebase this PR.

iloveeclipse · 2025-03-24T10:43:54Z

Let rebase this PR.

... and finally we have a green state!

iloveeclipse

I will handle search_enabled vs searchEnabled inconsistent naming in a follow up PR.

bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java

Follow up on eclipse-platform#1829 review. Renamed the `search_enabled` method to have same name / visibility as on Tree/Table classes.

Follow up on #1829 review. Renamed the `search_enabled` method to have same name / visibility as on Tree/Table classes.

chrstg · 2025-03-24T12:48:12Z

thanks a lot. Great!

Follow up on eclipse-platform#1829 review. Renamed the `search_enabled` method to have same name / visibility as on Tree/Table classes.

iloveeclipse requested changes Feb 14, 2025

View reviewed changes

chrstg mentioned this pull request Feb 14, 2025

Provide an option to disable the search dialog via Strg-F on list and tables #1830

Closed

iloveeclipse reviewed Feb 14, 2025

View reviewed changes

bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/SWT.java Outdated Show resolved Hide resolved

chrstg force-pushed the master branch 3 times, most recently from 78e1d5d to ae774d9 Compare February 14, 2025 15:12

chrstg requested a review from iloveeclipse February 14, 2025 15:21

chrstg force-pushed the master branch from 1e97599 to 283b38e Compare February 17, 2025 09:47

chrstg force-pushed the master branch 4 times, most recently from 80cf69b to d6c80a4 Compare February 18, 2025 08:16

chrstg force-pushed the master branch 2 times, most recently from 081d132 to d7d4436 Compare February 18, 2025 08:39

chrstg force-pushed the master branch 4 times, most recently from b19cb9c to 32f3104 Compare March 19, 2025 11:59

iloveeclipse reviewed Mar 20, 2025

View reviewed changes

chrstg force-pushed the master branch 5 times, most recently from 890323b to b30e803 Compare March 21, 2025 14:46

iloveeclipse mentioned this pull request Mar 24, 2025

DO NOT MERGE #1931

Closed

iloveeclipse added a commit to iloveeclipse/eclipse.platform.swt that referenced this pull request Mar 24, 2025

DO NOT MERGE

e0f8a0a

Just a test if we can rebuild natives on gtk.linux.riscv64 See eclipse-platform#1829

Provide an option to disable the search dialog via Strg-F on list and

f213a14

tables eclipse-platform#1830 add SWT.NO_SEARCH flag used in Table, Tree and List to prevent the interactive search (popup) in a GTK environment when set to true.

iloveeclipse force-pushed the master branch from b30e803 to f213a14 Compare March 24, 2025 10:13

iloveeclipse approved these changes Mar 24, 2025

View reviewed changes

bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java Show resolved Hide resolved

iloveeclipse merged commit ccbd39c into eclipse-platform:master Mar 24, 2025
14 checks passed

iloveeclipse mentioned this pull request Mar 24, 2025

Renamed search_enabled to searchEnabled to be consistent #1934

Merged

iloveeclipse added a commit that referenced this pull request Mar 24, 2025

Renamed search_enabled to searchEnabled to be consistent

0b7090f

Follow up on #1829 review. Renamed the `search_enabled` method to have same name / visibility as on Tree/Table classes.

akurtakov mentioned this pull request May 19, 2025

SWT Javadoc bash for 4.36 #2161

Merged

possibility to suppress GTK interactive search (popup) #1829

possibility to suppress GTK interactive search (popup) #1829

Uh oh!

Conversation

chrstg commented Feb 14, 2025

Uh oh!

iloveeclipse left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

chrstg commented Feb 17, 2025

Uh oh!

iloveeclipse commented Feb 17, 2025

Uh oh!

chrstg commented Feb 17, 2025

Uh oh!

iloveeclipse commented Feb 17, 2025

Uh oh!

chrstg commented Feb 17, 2025

Uh oh!

iloveeclipse commented Feb 17, 2025

Uh oh!

merks commented Feb 17, 2025

Uh oh!

chrstg commented Feb 17, 2025

Uh oh!

chrstg commented Feb 17, 2025

Uh oh!

merks commented Feb 17, 2025

Uh oh!

chrstg commented Feb 18, 2025

Uh oh!

chrstg commented Feb 18, 2025

Uh oh!

merks commented Feb 18, 2025

Uh oh!

merks commented Feb 18, 2025

Uh oh!

chrstg commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrstg commented Mar 19, 2025

Uh oh!

iloveeclipse Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

chrstg Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

iloveeclipse commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrstg commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iloveeclipse commented Mar 21, 2025

Uh oh!

chrst4 commented Mar 24, 2025

Uh oh!

iloveeclipse commented Mar 24, 2025

Uh oh!

iloveeclipse commented Mar 24, 2025

Uh oh!

iloveeclipse commented Mar 24, 2025

Uh oh!

iloveeclipse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chrstg commented Mar 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

github-actions bot commented Feb 14, 2025 •

edited

Loading

chrstg commented Feb 18, 2025 •

edited

Loading

iloveeclipse commented Mar 20, 2025 •

edited

Loading

chrstg commented Mar 21, 2025 •

edited

Loading