Skip to content
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

Move filters Button in Call Hierarchy view to the top level Bar #1671

Merged
merged 2 commits into from
Oct 4, 2024
Merged

Move filters Button in Call Hierarchy view to the top level Bar #1671

merged 2 commits into from
Oct 4, 2024

Conversation

jannisCode
Copy link
Contributor

What it does

Before the changes the filters could only be accessed through the "View Menu" (three dots), which is inconvenient considering that the filters button is rather important and frequently used.
After the changes the filters can be accessed by clicking on the icon in the top level bar as well as the old way through the view menu.

Before

image

After

Ticket1_bearbeitet

Author checklist

fedejeanne
fedejeanne suggested changes Sep 25, 2024
View reviewed changes
Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

@jannisCode thank you for the contribution, it looks good and works as expected!

I have some minor change requests though, mostly regarding new lines but the important one is about extracting the internal class org.eclipse.jdt.internal.ui.callhierarchy.CallHierarchyFiltersActionGroup.ShowFilterDialogAction in order to reduce code duplication.

org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/FiltersDialog.java Outdated Show resolved Hide resolved
org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/FiltersAction.java Outdated Show resolved Hide resolved
org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/FiltersAction.java Outdated Show resolved Hide resolved
org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/FiltersAction.java Outdated Show resolved Hide resolved
org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/CallHierarchyViewPart.java Outdated Show resolved Hide resolved
@jannisCode jannisCode force-pushed the filters_action branch 5 times, most recently from 8512f00 to 8c1c2f9 Compare September 27, 2024 08:24
@jannisCode jannisCode force-pushed the filters_action branch 2 times, most recently from 109872e to 5c6bf32 Compare October 1, 2024 08:37
@jannisCode jannisCode changed the title Move filters Button in Call Hierarchy view to the top level Bar Bump Versions Oct 1, 2024
@jannisCode jannisCode changed the title Bump Versions Move filters Button in Call Hierarchy view to the top level Bar Oct 1, 2024
@fedejeanne
Copy link
Contributor

@jannisCode regarding this build error:

10:45:23  [ERROR] Failed to execute goal org.eclipse.tycho:tycho-packaging-plugin:4.0.9:validate-version (default-validate-version) on project org.eclipse.jdt.ui: Unqualified OSGi version 3.33.200.qualifier must match unqualified Maven version 3.33.100-SNAPSHOT for SNAPSHOT builds -> [Help 1]
10:45:23  org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.eclipse.tycho:tycho-packaging-plugin:4.0.9:validate-version (default-validate-version) on project org.eclipse.jdt.ui: Unqualified OSGi version 3.33.200.qualifier must match unqualified Maven version 3.33.100-SNAPSHOT for SNAPSHOT builds

It seems that the pom.xml of org.eclipse.jdt.ui also has to be updated (increase the version too). Please do it as part of the same commit were you bumped the MANIFEST.MF (amend).

@fedejeanne
Copy link
Contributor

fedejeanne commented Oct 2, 2024 •
edited
Loading

Thank you for addressing my change request @jannisCode . The PR looks good for me but since I'm not a committer, I can't merge it.

Maybe some committer (@jjohnstn / @vogella?) wants to take a look and let the workflows run before reviewing/merging?

@jannisCode @akurtakov
Move filters Button in Call Hierarchy view to the top level Bar
efb064d 
- Added class FiltersAction
- Added fFiltersAction to CallHierarchyViewPart to create an icon in the
top level bar
- changed the constructor of FiltersAction so that the picture and Text
are the same as in the View Menu
- Connected the icon to its functionality by adding a method called
openFiltersDialog which opens the pop up window for the filters
@jannisCode @akurtakov
Bump Versions
ec21387
@vogella
Copy link
Contributor

vogella commented Oct 2, 2024

@fedejeanne I'm not a JDT committer

@jjohnstn
Copy link
Contributor

jjohnstn commented Oct 4, 2024

@fedejeanne I'll look at it today.

@jjohnstn jjohnstn self-requested a review October 4, 2024 16:59
@jjohnstn jjohnstn added the enhancement New feature or request label Oct 4, 2024
jjohnstn
jjohnstn approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@jjohnstn jjohnstn left a comment

Choose a reason for hiding this comment

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

Looks good (all of @fedejeanne comments have been addressed) and works fine.

@jjohnstn jjohnstn merged commit 68f1142 into eclipse-jdt:master Oct 4, 2024
7 of 10 checks passed
@jjohnstn jjohnstn added the noteworthy Noteworthy feature label Oct 4, 2024
@jjohnstn
Copy link
Contributor

jjohnstn commented Oct 4, 2024

Hi @jannisCode Thank you for the patch.

@merks
Copy link
Contributor

merks commented Oct 5, 2024

@jjohnstn @jannisCode

After a pull, my IDE now has this error:

image

Indeed it was incremented 4 weeks ago:

image

I don't think we can undo the damage after this has been pulled into an I-Build.

@merks merks mentioned this pull request Oct 5, 2024
Merged
merks added a commit to merks/eclipse.jdt.ui that referenced this pull request Oct 5, 2024
@merks
Filter problem of accidental extra version increment
4c9c2e9 
eclipse-jdt#1671
iloveeclipse pushed a commit that referenced this pull request Oct 7, 2024
@merks @iloveeclipse
Filter problem of accidental extra version increment
8f8fafd 
#1671
@fedejeanne
Copy link
Contributor

@merks thank you for reporting it. Do you know why was this not reported by the (GH) builds?

I noticed the same error locally when trying to fix the build in this PR but I thought it might be a false positive in my IDE. Increasing the version fixed the PR-builds. More details in my discussion (I'd like to continue the discussion there): eclipse-platform/eclipse.platform.releng.aggregator#2406

This was referenced Oct 11, 2024
Closed
Merged
Open
@jannisCode jannisCode mentioned this pull request Oct 22, 2024
Draft
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjohnstn jjohnstn jjohnstn approved these changes

@fedejeanne fedejeanne fedejeanne approved these changes

@Wittmaxi Wittmaxi Wittmaxi approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request noteworthy Noteworthy feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jannisCode @fedejeanne @vogella @jjohnstn @merks @Wittmaxi