Skip to content

Added toggle for filtering failed tasks #1325

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added toggle for filtering failed tasks #1325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41fc995
Added toggle for filtering failed tasks
Tanish-Ranjan Feb 9, 2025
6d3bc9d
Merge branch 'eclipse-buildship:master' into feat/filter_failed_tests
Tanish-Ranjan Mar 11, 2025
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
1 change: 1 addition & 0 deletions ...eclipse.buildship.ui/src/main/java/org/eclipse/buildship/ui/internal/i18n/UiMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public final class UiMessages extends NLS {

public static String Button_Label_Browse;

public static String Action_FilterFailedTasks_Tooltip;
Copy link
Contributor

Choose a reason for hiding this comment

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

Name should be adjusted.

public static String Action_ExpandNodes_Tooltip;
public static String Action_CollapseNodes_Tooltip;
public static String Action_ShowFilter_Tooltip;
Expand Down
97 changes: 97 additions & 0 deletions ...p.ui/src/main/java/org/eclipse/buildship/ui/internal/view/ExpandAllFailedTasksAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*******************************************************************************
* Copyright (c) 2023 Gradle Inc. and others
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
******************************************************************************/
package org.eclipse.buildship.ui.internal.view;

import org.gradle.tooling.events.FailureResult;
import org.gradle.tooling.events.FinishEvent;
import org.gradle.tooling.events.OperationResult;

import com.google.common.base.Preconditions;

import org.eclipse.jface.action.Action;
import org.eclipse.jface.viewers.AbstractTreeViewer;

import org.eclipse.buildship.ui.internal.PluginImages;
import org.eclipse.buildship.ui.internal.PluginImage.ImageState;
import org.eclipse.buildship.ui.internal.i18n.UiMessages;
import org.eclipse.buildship.ui.internal.util.nodeselection.NodeSelection;
import org.eclipse.buildship.ui.internal.util.nodeselection.SelectionSpecificAction;
import org.eclipse.buildship.ui.internal.view.execution.ExecutionPageContentProvider;
import org.eclipse.buildship.ui.internal.view.execution.OperationItem;

/**
*
*/
public final class ExpandAllFailedTasksAction extends Action implements SelectionSpecificAction {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public final class ExpandAllFailedTasksAction extends Action implements SelectionSpecificAction {
public final class ExpandFailuresAction extends Action implements SelectionSpecificAction {

❌ the name doesn't correspond to the functionality. We collapse the tree and expand the nodes representing failed progress events.


private final AbstractTreeViewer treeViewer;
private ExecutionPageContentProvider contentProvider = null;

public ExpandAllFailedTasksAction(AbstractTreeViewer treeViewer) {
super(null, AS_CHECK_BOX);
this.treeViewer = Preconditions.checkNotNull(treeViewer);


setToolTipText(UiMessages.Action_FilterFailedTasks_Tooltip);
setImageDescriptor(PluginImages.OPERATION_FAILURE.withState(ImageState.ENABLED).getImageDescriptor());
}

public void setContentProvider(ExecutionPageContentProvider contentProvider) {
this.contentProvider = contentProvider;
setChecked(contentProvider.isFilterFailedItemsEnabled());
}

@Override
public void run() {

this.contentProvider.toggleFilterFailedItems();
setChecked(this.contentProvider.isFilterFailedItemsEnabled());

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

this.treeViewer.collapseAll();

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Object rootObject = this.treeViewer.getInput();

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

if (rootObject instanceof OperationItem) {
OperationItem root = (OperationItem) rootObject;
recursivelyExpand(root);
}

}

private void recursivelyExpand(OperationItem parent) {
FinishEvent event = parent.getFinishEvent();
if (event != null) {
OperationResult result = event.getResult();
if (result instanceof FailureResult) {
// Result failed (expand ancestors up to this element)
this.treeViewer.expandToLevel(parent, 0);
}
}
for (OperationItem item: parent.getChildren()) {
recursivelyExpand(item);
}
}

@Override
public boolean isVisibleFor(NodeSelection selection) {
return true;
}

@Override
public boolean isEnabledFor(NodeSelection selection) {
return true;
}

@Override
public void setEnabledFor(NodeSelection selection) {
setEnabled(this.contentProvider != null);
}

}
4 changes: 4 additions & 0 deletions ...ship.ui/src/main/java/org/eclipse/buildship/ui/internal/view/execution/ExecutionPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.eclipse.buildship.ui.internal.util.widget.PatternFilter;
import org.eclipse.buildship.ui.internal.view.BasePage;
import org.eclipse.buildship.ui.internal.view.CollapseAllTreeNodesAction;
import org.eclipse.buildship.ui.internal.view.ExpandAllFailedTasksAction;
import org.eclipse.buildship.ui.internal.view.ExpandAllTreeNodesAction;
import org.eclipse.buildship.ui.internal.view.MultiPageView;
import org.eclipse.buildship.ui.internal.view.PageSite;
Expand Down Expand Up @@ -290,6 +291,9 @@ private void populateToolBar() {
toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, new ExpandAllTreeNodesAction(getPageControl().getViewer()));
toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, new CollapseAllTreeNodesAction(getPageControl().getViewer()));
toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, new ShowFilterAction(getPageControl()));
ExpandAllFailedTasksAction action = new ExpandAllFailedTasksAction(getPageControl().getViewer());
action.setContentProvider((ExecutionPageContentProvider)getPageControl().getViewer().getContentProvider());
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ this can happen in the action's constructor. Then, you can use the same pattern as around, ie

toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, new ExpandAllFailedTasksAction(...));

Copy link
Author

@Tanish-Ranjan Tanish-Ranjan Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't add ExecutionPageContentProvider as a constructor parameter, since UiContributionManager also takes an instance of the action and I don't have access to the content provider there.

Copy link
Contributor

Choose a reason for hiding this comment

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

By the looks of it, UiContributionManager contributes actions to the tasks view. In other words it's independent from the (execution) view this PR aims to improve. You probably don't need to modify it whatsoever.

Copy link
Author

Choose a reason for hiding this comment

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

Okay

toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, action);
toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, new Separator());
toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, new SwitchToConsoleViewAction(this));
toolbarManager.appendToGroup(MultiPageView.PAGE_GROUP, new Separator());
Expand Down
24 changes: 23 additions & 1 deletion ...n/java/org/eclipse/buildship/ui/internal/view/execution/ExecutionPageContentProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
******************************************************************************/
package org.eclipse.buildship.ui.internal.view.execution;

import java.util.stream.Collectors;

import org.gradle.tooling.events.FailureResult;

import org.eclipse.jface.viewers.ITreeContentProvider;
import org.eclipse.jface.viewers.Viewer;

Expand All @@ -17,14 +21,24 @@
*/
public class ExecutionPageContentProvider implements ITreeContentProvider {

private boolean filterFailedItems = false;
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 wrong as the state will get lost when the IDE restarts. Instead, store the state in WorkspaceConfiguration. Then you can initialize the action from the worksace configuration and update it upon a toggle action.

Copy link
Contributor

Choose a reason for hiding this comment

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

Scratch that, the proper place to store the state is ExecutionViewState.


@Override
public Object[] getElements(Object inputElement) {
return getChildren(inputElement);
}

@Override
public Object[] getChildren(Object parent) {
return parent instanceof OperationItem ? ((OperationItem)parent).getChildren().toArray() : new Object[0];
if (this.isFilterFailedItemsEnabled()) {
if (parent instanceof OperationItem) {
return ((OperationItem)parent).getChildren().stream().filter(operationItem -> operationItem.getFinishEvent().getResult() instanceof FailureResult).collect(Collectors.toList()).toArray();
} else {
return new Object[0];
}
} else {
return parent instanceof OperationItem ? ((OperationItem)parent).getChildren().toArray() : new Object[0];
}
}

@Override
Expand All @@ -44,4 +58,12 @@ public void inputChanged(Viewer viewer, Object oldInput, Object newInput) {
@Override
public void dispose() {
}

public boolean isFilterFailedItemsEnabled() {
return this.filterFailedItems;
}

public void toggleFilterFailedItems() {
this.filterFailedItems = !this.filterFailedItems;
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ Please don't do this. Use explicit getters and setters.

}
}
2 changes: 2 additions & 0 deletions ...p.ui/src/main/java/org/eclipse/buildship/ui/internal/view/task/UiContributionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.eclipse.buildship.ui.internal.util.nodeselection.SelectionSpecificAction;
import org.eclipse.buildship.ui.internal.util.selection.ContextActivatingViewPartListener;
import org.eclipse.buildship.ui.internal.view.CollapseAllTreeNodesAction;
import org.eclipse.buildship.ui.internal.view.ExpandAllFailedTasksAction;
import org.eclipse.buildship.ui.internal.view.ExpandAllTreeNodesAction;
import org.eclipse.buildship.ui.internal.view.ShowFilterAction;

Expand Down Expand Up @@ -94,6 +95,7 @@ public void wire() {
private void populateToolBar() {
IToolBarManager manager = this.taskView.getViewSite().getActionBars().getToolBarManager();
manager.add(new GroupMarker(TOOLBAR_TREE_GROUP));
manager.appendToGroup(TOOLBAR_TREE_GROUP, new ExpandAllFailedTasksAction(this.taskView.getTreeViewer()));
manager.appendToGroup(TOOLBAR_TREE_GROUP, new ExpandAllTreeNodesAction(this.taskView.getTreeViewer()));
manager.appendToGroup(TOOLBAR_TREE_GROUP, new CollapseAllTreeNodesAction(this.taskView.getTreeViewer()));
manager.appendToGroup(TOOLBAR_TREE_GROUP, new ShowFilterAction(this.taskView.getFilteredTree()));
Expand Down
1 change: 1 addition & 0 deletions ...ldship.ui/src/main/resources/org/eclipse/buildship/ui/internal/i18n/UiMessages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Title_Gradle_Build_Script_Compare=Gradle Build Script Compare

Button_Label_Browse=Browse...

Action_FilterFailedTasks_Tooltip=Filter Failed Tasks
Copy link
Contributor

Choose a reason for hiding this comment

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

❌

Suggested change
Action_FilterFailedTasks_Tooltip=Filter Failed Tasks
Action_FilterFailedTasks_Tooltip=Expand Failures

Action_ExpandNodes_Tooltip=Expand All
Action_CollapseNodes_Tooltip=Collapse All
Action_ShowFilter_Tooltip=Show Filter
Expand Down