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

Refactoring Filter from call Hierarchy #1730

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactoring Filter from call Hierarchy #1730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions ...ation/core extension/org/eclipse/jdt/internal/corext/callhierarchy/CallHierarchyCore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@
import org.eclipse.jdt.internal.ui.util.StringMatcher;

public class CallHierarchyCore {

public static final String PREF_SHOW_ALL_CODE = "PREF_SHOW_ALL_CODE"; //$NON-NLS-1$
public static final String PREF_HIDE_TEST_CODE = "PREF_HIDE_TEST_CODE"; //$NON-NLS-1$
public static final String PREF_SHOW_TEST_CODE_ONLY = "PREF_SHOW_TEST_CODE_ONLY"; //$NON-NLS-1$

public static final String PREF_USE_IMPLEMENTORS= "PREF_USE_IMPLEMENTORS"; //$NON-NLS-1$
public static final String PREF_USE_FILTERS= "PREF_USE_FILTERS"; //$NON-NLS-1$
public static final String PREF_FILTERS_LIST= "PREF_FILTERS_LIST"; //$NON-NLS-1$
Expand All @@ -72,15 +67,23 @@ public boolean isSearchUsingImplementorsEnabled() {
}

public boolean isShowTestCode() {
return Boolean.parseBoolean(JavaManipulation.getPreference(PREF_SHOW_TEST_CODE_ONLY, null));
return Boolean.parseBoolean(JavaManipulation.getPreference(CallHierarchyFilterOptions.SHOW_TEST_CODE_ONLY.getId(), null));
}

public boolean isShowAll() {
return Boolean.parseBoolean(JavaManipulation.getPreference(PREF_SHOW_ALL_CODE, null));
return Boolean.parseBoolean(JavaManipulation.getPreference(CallHierarchyFilterOptions.SHOW_ALL_CODE.getId(), null));
}

public boolean isHideTestCode() {
return Boolean.parseBoolean(JavaManipulation.getPreference(PREF_HIDE_TEST_CODE, null));
return Boolean.parseBoolean(JavaManipulation.getPreference(CallHierarchyFilterOptions.HIDE_TEST_CODE.getId(), null));
}

public String getActiveFilter() {
for (CallHierarchyFilterOptions option: CallHierarchyFilterOptions.values()) { //must be one of the threee
if(Boolean.parseBoolean(JavaManipulation.getPreference(option.getId(), null))) {
return option.getId();
}
} return null;
}

public Collection<IJavaElement> getImplementingMethods(IMethod method) {
Expand Down
41 changes: 41 additions & 0 deletions ...e extension/org/eclipse/jdt/internal/corext/callhierarchy/CallHierarchyFilterOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*******************************************************************************
* Copyright (c) 2024 Vector Informatik GmbH and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Vector Informatik GmbH - initial API and implementation
*******************************************************************************/

package org.eclipse.jdt.internal.corext.callhierarchy;

/**
* These are the filter Options for the Call Hierarchy
* When adding one, modify the getFilterOptions Method in FilterOptions
*/
public enum CallHierarchyFilterOptions {
SHOW_ALL_CODE("PREF_SHOW_ALL_CODE", CallHierarchyMessages.FiltersDialog_ShowAllCode), //$NON-NLS-1$
HIDE_TEST_CODE("PREF_HIDE_TEST_CODE", CallHierarchyMessages.FiltersDialog_HideTestCode), //$NON-NLS-1$
SHOW_TEST_CODE_ONLY("PREF_SHOW_TEST_CODE_ONLY", CallHierarchyMessages.FiltersDialog_TestCodeOnly); //$NON-NLS-1$

private final String identifyString;
private final String text;

CallHierarchyFilterOptions(String identifyString, String text) {
this.identifyString = identifyString;
this.text = text;
}

public String getId() {
return identifyString;
}

public String getText() {
return text;
}
}
4 changes: 4 additions & 0 deletions ...n/core extension/org/eclipse/jdt/internal/corext/callhierarchy/CallHierarchyMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ private CallHierarchyMessages() {

public static String CallerMethodWrapper_taskname;
public static String CalleeMethodWrapper_taskname;
public static String FiltersDialog_ShowAllCode;
public static String FiltersDialog_HideTestCode;
public static String FiltersDialog_TestCodeOnly;


static {
NLS.initializeMessages(BUNDLE_NAME, CallHierarchyMessages.class);
Expand Down
4 changes: 4 additions & 0 deletions ... extension/org/eclipse/jdt/internal/corext/callhierarchy/CallHierarchyMessages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@
###############################################################################
CallerMethodWrapper_taskname=Finding callers...
CalleeMethodWrapper_taskname=Finding callees...

FiltersDialog_HideTestCode = Hide Test Code
FiltersDialog_ShowAllCode = Show All Code
FiltersDialog_TestCodeOnly = Test Code only
43 changes: 14 additions & 29 deletions ...se.jdt.ui/core extension/org/eclipse/jdt/internal/corext/callhierarchy/CallHierarchy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
package org.eclipse.jdt.internal.corext.callhierarchy;

import static org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchyCore.PREF_FILTERS_LIST;
import static org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchyCore.PREF_HIDE_TEST_CODE;
import static org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchyCore.PREF_SHOW_ALL_CODE;
import static org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchyCore.PREF_SHOW_TEST_CODE_ONLY;
import static org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchyCore.PREF_USE_FILTERS;
import static org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchyCore.PREF_USE_IMPLEMENTORS;

Expand All @@ -36,6 +33,7 @@
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.dom.ASTParser;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.manipulation.JavaManipulation;
import org.eclipse.jdt.core.search.IJavaSearchScope;

import org.eclipse.jdt.internal.corext.dom.IASTSharedValues;
Expand All @@ -60,20 +58,14 @@ public static CallHierarchy getDefault() {
return fgInstance;
}

public void setShowAll(boolean value) {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
settings.setValue(PREF_SHOW_ALL_CODE, value);
}
public void setActiveFilter(CallHierarchyFilterOptions filter) {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();

public void setHideTestCode(boolean value) {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
settings.setValue(PREF_HIDE_TEST_CODE, value);
for(CallHierarchyFilterOptions option : CallHierarchyFilterOptions.values()) {
settings.setValue(option.getId(), option == filter);
}
}

public void setShowTestCode(boolean value) {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
settings.setValue(PREF_SHOW_TEST_CODE_ONLY, value);
}

public boolean isSearchUsingImplementorsEnabled() {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
Expand Down Expand Up @@ -139,21 +131,14 @@ public boolean isFilterEnabled() {
return settings.getBoolean(PREF_USE_FILTERS);
}

public boolean isShowAll() {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
return settings.getBoolean(PREF_SHOW_ALL_CODE);
}

public boolean isHideTestCode() {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
return settings.getBoolean(PREF_HIDE_TEST_CODE);
}

public boolean isShowTestCode() {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
return settings.getBoolean(PREF_SHOW_TEST_CODE_ONLY);
}

public CallHierarchyFilterOptions getActiveFilter() {
for (CallHierarchyFilterOptions option: CallHierarchyFilterOptions.values()) {
if(Boolean.parseBoolean(JavaManipulation.getPreference(option.getId(), null))) {
return option;
}
}
return null;
}

public void setFilterEnabled(boolean filterEnabled) {
IPreferenceStore settings = JavaPlugin.getDefault().getPreferenceStore();
Expand Down
4 changes: 0 additions & 4 deletions org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/CallHierarchyMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ private CallHierarchyMessages() {
public static String ShowFilterDialogAction_text;
public static String FiltersDialog_filter;

public static String FiltersDialog_ShowAllCode;
public static String FiltersDialog_HideTestCode;
public static String FiltersDialog_TestCodeOnly;

public static String FiltersDialog_filterOnNames;
public static String FiltersDialog_filterOnNamesSubCaption;
public static String FiltersDialog_maxCallDepth;
Expand Down
4 changes: 0 additions & 4 deletions ...ipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/CallHierarchyMessages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ SearchInDialog_title= Search In
ShowExpandWithConstructorsDialogAction_text=E&xpand with Constructors...
ShowFilterDialogAction_text= &Filters...

FiltersDialog_HideTestCode = Hide Test Code
FiltersDialog_ShowAllCode = Show All Code
FiltersDialog_TestCodeOnly = Test Code only

FiltersDialog_filter= Filter Calls
FiltersDialog_filterOnNames= &Name filter patterns (matching names will be hidden):
FiltersDialog_filterOnNamesSubCaption= Patterns are separated by commas (* = any string, ? = any character)
Expand Down
50 changes: 29 additions & 21 deletions org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/callhierarchy/FiltersDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
*******************************************************************************/
package org.eclipse.jdt.internal.ui.callhierarchy;

import java.util.ArrayList;
import java.util.List;

import org.eclipse.swt.SWT;
import org.eclipse.swt.events.SelectionAdapter;
import org.eclipse.swt.events.SelectionEvent;
Expand All @@ -31,18 +34,18 @@
import org.eclipse.ui.PlatformUI;

import org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchy;
import org.eclipse.jdt.internal.corext.callhierarchy.CallHierarchyFilterOptions;

import org.eclipse.jdt.internal.ui.IJavaHelpContextIds;
import org.eclipse.jdt.internal.ui.dialogs.StatusInfo;

class FiltersDialog extends StatusDialog {
private Label fNamesHelpText;
private Button fFilterOnNames;
Copy link
Contributor

Choose a reason for hiding this comment

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

You need this one if you follow my advice below:

Suggested change
private Button fFilterOnNames;
private static final String KEY_CALL_HIERARCHY_FILTER_OPTION= "callHierarchyFilterOption"; //$NON-NLS-1$
private Button fFilterOnNames;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

private static final String KEY_CALL_HIERARCHY_FILTER_OPTION= "callHierarchyFilterOption"; //$NON-NLS-1$
private Text fNames;
private Text fMaxCallDepth;
private Button fShowAll;
private Button fHideTest;
private Button fShowTest;
private List<Button> buttons = new ArrayList<>(CallHierarchyFilterOptions.values().length); //important what comes when

protected FiltersDialog(Shell parentShell) {
super(parentShell);
Expand Down Expand Up @@ -118,28 +121,27 @@ private void createTestCodeArea(Composite parent) {
layout.numColumns= 1;
radioGroup.setLayout(layout);

fShowAll= new Button(radioGroup, SWT.RADIO);
fShowAll.setText(CallHierarchyMessages.FiltersDialog_ShowAllCode);

fHideTest= new Button(radioGroup, SWT.RADIO);
fHideTest.setText(CallHierarchyMessages.FiltersDialog_HideTestCode);
for (CallHierarchyFilterOptions op : CallHierarchyFilterOptions.values()) {
Button b= new Button(radioGroup, SWT.RADIO);
b.setText(op.getText());
b.setData(KEY_CALL_HIERARCHY_FILTER_OPTION, op);
buttons.add(b);
}

fShowTest= new Button(radioGroup, SWT.RADIO);
fShowTest.setText(CallHierarchyMessages.FiltersDialog_TestCodeOnly);
setSelection();

GridData gridData= new GridData();
gridData.horizontalIndent= 0;
fShowAll.setLayoutData(gridData);
fHideTest.setLayoutData(gridData);
fShowTest.setLayoutData(gridData);

for (Button button : buttons) {
button.setLayoutData(gridData);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rewrite this function to this:

Suggested change
}
private void createTestCodeArea(Composite parent) {
Composite radioGroup= new Composite(parent, SWT.NONE);
radioGroup.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
GridLayout layout= new GridLayout();
layout.numColumns= 1;
radioGroup.setLayout(layout);
GridData gridData= new GridData(GridData.FILL_HORIZONTAL);
gridData.horizontalIndent= 0;
for (CallHierarchyFilterOptions op : CallHierarchyFilterOptions.values()) {
Button b= new Button(radioGroup, SWT.RADIO);
b.setText(op.getText());
b.setData(KEY_CALL_HIERARCHY_FILTER_OPTION, op);
b.setLayoutData(gridData);
buttons.add(b);
}
setSelection();
}

That would fix the trimming of the labels too.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a refactoring, it adds/changes the following lines:

  • radioGroup.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
  • GridData gridData= new GridData(GridData.FILL_HORIZONTAL);
  • b.setLayoutData(gridData);

This should be done in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be done in another PR.

Would that be OK @gzsombor ?


private void setSelection() {
fShowAll.setSelection(CallHierarchy.getDefault().isShowAll());
fHideTest.setSelection(CallHierarchy.getDefault().isHideTestCode());
fShowTest.setSelection(CallHierarchy.getDefault().isShowTestCode());

for (Button button : buttons) {
button.setSelection(CallHierarchy.getDefault().getActiveFilter() == getFilterOptions(button));
}
}

/**
Expand Down Expand Up @@ -191,9 +193,13 @@ private void updateFilterFromUI() {
CallHierarchy.getDefault().setFilters(fNames.getText());
CallHierarchy.getDefault().setFilterEnabled(fFilterOnNames.getSelection());

CallHierarchy.getDefault().setShowAll(fShowAll.getSelection());
CallHierarchy.getDefault().setHideTestCode(fHideTest.getSelection());
CallHierarchy.getDefault().setShowTestCode(fShowTest.getSelection());
CallHierarchyFilterOptions activeFilter = null;
for (Button button : buttons) {
if(button.getSelection()) {
activeFilter = getFilterOptions(button);
}
}
CallHierarchy.getDefault().setActiveFilter(activeFilter);
}

/**
Expand All @@ -205,10 +211,12 @@ private void updateUIFromFilter() {
fFilterOnNames.setSelection(CallHierarchy.getDefault().isFilterEnabled());

setSelection();

updateEnabledState();
}

private CallHierarchyFilterOptions getFilterOptions(Button b) {
return (CallHierarchyFilterOptions) b.getData(KEY_CALL_HIERARCHY_FILTER_OPTION);
}

/**
* Updates the filter from the UI state.
Expand Down
Loading