-
Notifications
You must be signed in to change notification settings - Fork 27
tmf.ui: Make AbstractSelectTreeViewer2 injectable; add AbstractSelectTreeViewer3 and SpecificColumnPatternFilter for column-aware filtering #366
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.
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughBumps TMF UI bundle and POM versions; relaxes a tree viewer constructor to protected; adds AbstractSelectTreeViewer3 which builds a multi-select, full-row TriStateFilteredCheckboxTree with a column-specific filter; makes MultiTreePatternFilter column index configurable and adds SpecificColumnPatternFilter. Changes
Sequence Diagram(s)sequenceDiagram
participant Parent as Composite (parent)
participant Viewer as AbstractSelectTreeViewer3
participant Tree as TriStateFilteredCheckboxTree
participant Filter as SpecificColumnPatternFilter
Parent ->> Viewer: new AbstractSelectTreeViewer3(parent, legendIndex, id, indexColumnFilter)
Viewer ->> Tree: create TriStateFilteredCheckboxTree(multi-select, full-row)
Viewer ->> Filter: new SpecificColumnPatternFilter(indexColumnFilter)
Viewer ->> Tree: installFilter(Filter)
Tree ->> Filter: evaluate rows using getIndexColumnFilter()
Note right of Tree: Filter applies patterns only on configured column
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (2)
27-27: Consider more descriptive field naming.The field name
indexis generic. Consider using a more descriptive name following the Eclipse/TraceCompass conventions, such asfColumnIndexorfIndexColumnFilter(withfprefix for fields).Apply this diff for improved clarity:
- private int index = 0; + private final int fColumnIndex;And update the constructor and getter:
public SpecificColumnPatternFilter(int indexColumnFilter) { - this.index = indexColumnFilter; + this.fColumnIndex = indexColumnFilter; } @Override protected int getIndexColumnFilter() { - return this.index; + return this.fColumnIndex; }
34-36: Consider validation for column index parameter.The constructor accepts any integer value for
indexColumnFilter, including negative values. While the Eclipse SWT framework might handle negative indices gracefully, consider adding validation to ensure the column index is non-negative or documenting the expected range.Add validation if negative indices are not supported:
public SpecificColumnPatternFilter(int indexColumnFilter) { + if (indexColumnFilter < 0) { + throw new IllegalArgumentException("Column index must be non-negative"); + } this.index = indexColumnFilter; }Alternatively, document the acceptable range in the Javadoc:
/** * Creates a new filter with the specified column index. - * @param indexColumnFilter the index of the column to apply the filter on + * @param indexColumnFilter the index of the column to apply the filter on (must be >= 0) */tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java (1)
44-44: Consider removing redundant inline comment.The inline comment "Initialize the tree viewer with a filtered checkbox tree and column-based filtering" restates what's already clear from the constructor's Javadoc and the code itself.
Apply this diff to remove the redundant comment:
- // Initialize the tree viewer with a filtered checkbox tree and column-based filtering super(parent, new TriStateFilteredCheckboxTree(parent,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF(3 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer2.java(1 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java(1 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/MultiTreePatternFilter.java(2 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java (2)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (1)
SpecificColumnPatternFilter(26-46)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/TriStateFilteredCheckboxTree.java (1)
TriStateFilteredCheckboxTree(43-249)
🔇 Additional comments (7)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer2.java (1)
223-224: LGTM! Constructor visibility change enables subclassing.Changing the constructor from
privatetoprotectedappropriately enables the extensibility required byAbstractSelectTreeViewer3without breaking the existing public API.tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/MultiTreePatternFilter.java (2)
77-78: LGTM! Refactoring enables column-specific filtering.The change from a hard-coded column index to
getIndexColumnFilter()appropriately enables subclasses to customize the filtering column while preserving the default behavior (column 0).
89-96: LGTM! Well-documented extensibility point.The new protected method provides a clean extensibility point for subclasses. The default return value of 0 maintains backward compatibility.
tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF (3)
5-5: LGTM! Version bump is appropriate.The version bump to 9.2.0 correctly reflects the addition of new public API elements (
getIndexColumnFilter(),SpecificColumnPatternFilter,AbstractSelectTreeViewer3).
156-156: LGTM! Optional import is appropriate.The
traceeventloggerpackage is correctly marked as optional, which is appropriate for an optional dependency used for logging/tracing purposes.
33-35: Verify necessity of new bundle dependencies.The added bundle dependencies (
org.eclipse.equinox.p2.uiandorg.eclipse.ui.browser) should be verified as required for the changes in this PR (column-aware filtering, tree viewer extensibility). Search the codebase for imports or usage of these packages in the modified Java files to confirm they support the new functionality, or remove them if unused.tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java (1)
43-48: LGTM! Clean extension of AbstractSelectTreeViewer2.The implementation correctly leverages the newly protected constructor of
AbstractSelectTreeViewer2and integratesSpecificColumnPatternFilterfor column-aware filtering. The constructor parameters are well-documented.
| /******************************************************************************* | ||
| * Copyright (c) 2017 Ericsson | ||
| * | ||
| * All rights reserved. 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 | ||
| *******************************************************************************/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update copyright year for new file.
The copyright year is listed as 2017, but this is a new file being added in 2025. Update the copyright to reflect the correct year (e.g., "Copyright (c) 2025 Ericsson").
Apply this diff to update the copyright:
-/*******************************************************************************
- * Copyright (c) 2017 Ericsson
- *
+/*******************************************************************************
+ * Copyright (c) 2025 Ericsson
+ *📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /******************************************************************************* | |
| * Copyright (c) 2017 Ericsson | |
| * | |
| * All rights reserved. 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 | |
| *******************************************************************************/ | |
| /******************************************************************************* | |
| * Copyright (c) 2025 Ericsson | |
| * | |
| * All rights reserved. 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 | |
| *******************************************************************************/ |
🤖 Prompt for AI Agents
In
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java
around lines 1 to 10, the file header lists "Copyright (c) 2017 Ericsson" but
this is a new file added in 2025; update the copyright year to 2025 (e.g.,
"Copyright (c) 2025 Ericsson") and ensure the SPDX identifier and license text
remain unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change the company name to your company, or you a an individual, Ericsson does not own the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MatthewKhouzam, thank you for the correction! You're absolutely right—the copyright holder should reflect the actual owner of the code, not Ericsson.
@ShadowK2494, please update the copyright header to replace "Ericsson" with either your company name (if this code is being contributed on behalf of your employer) or your own name (if you're contributing as an individual). Also update the year from 2017 to 2025, since this is a new file.
For example:
/*******************************************************************************
* Copyright (c) 2025 [Your Company Name or Your Name]
*
* All rights reserved. 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
*******************************************************************************/|
Hi @ShadowK2494, Thank you for the PR, we will look at it soon, but until then, could you please sign the ECA? It is an essential step in our way of working, to make sure things are clean. |
| @@ -0,0 +1,49 @@ | |||
| /******************************************************************************* | |||
| * Copyright (c) 2017, 2025 Ericsson, Draeger Auriga | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder! I have signed the ECA and updated the commit as requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (1)
1-10: Fix copyright year and holder in header.This is a new file added in 2025, but the header says
Copyright (c) 2015 Hong Anh. Update the year to 2025 and ensure the holder matches the actual copyright owner (your name or your company, per how you’re contributing under the ECA).For example:
/******************************************************************************* - * Copyright (c) 2015 Hong Anh + * Copyright (c) 2025 Your Name or Your Company *Keep the EPL-2.0 text and SPDX identifier unchanged.
🧹 Nitpick comments (2)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (2)
26-36: Make the column index field immutable.
indexis set only in the constructor and never changed; making itfinalbetter expresses intent and avoids accidental reassignment later.-public class SpecificColumnPatternFilter extends MultiTreePatternFilter { - private int index = 0; +public class SpecificColumnPatternFilter extends MultiTreePatternFilter { + private final int index; @@ public SpecificColumnPatternFilter(int indexColumnFilter) { this.index = indexColumnFilter; }
29-32: Clarify column index semantics in Javadoc.Consider stating explicitly that the column index is 0‑based so callers don’t have to infer it from the underlying Tree APIs.
For example:
- * @param indexColumnFilter the index of the column to apply the filter on + * @param indexColumnFilter the 0-based index of the column to apply the filter on
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java(1 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (1)
26-45: Consider validating the column index and making it immutable.Functionally this is fine, but two small improvements would harden the API:
- Guard against invalid indices (e.g., negative values), which are unlikely to be meaningful as column indices.
- Make
indexfinalsince it’s set only in the constructor.For example:
-public class SpecificColumnPatternFilter extends MultiTreePatternFilter { - private int index = 0; +public class SpecificColumnPatternFilter extends MultiTreePatternFilter { + private final int index; @@ - public SpecificColumnPatternFilter(int indexColumnFilter) { - this.index = indexColumnFilter; + public SpecificColumnPatternFilter(int indexColumnFilter) { + if (indexColumnFilter < 0) { + throw new IllegalArgumentException("indexColumnFilter must be >= 0"); + } + this.index = indexColumnFilter; }This keeps usage explicit and fails fast on accidental misuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java(1 hunks)
🔇 Additional comments (1)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (1)
1-10: Header now matches contribution (year and author).License header looks correct for a new 2025 file and uses the proper EPL-2.0 SPDX identifier. Nothing to change here.
…TreeViewer3 and MultiTreePatternFilter2 for column-aware filtering Signed-off-by: Hong Anh <honganh.book@gmail.com>
5d66467 to
667b23e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (2)
27-27: Consider making the field final and removing redundant initialization.The
indexfield is set in the constructor and never modified afterward. Making itfinalwould better express this immutability and prevent accidental modification.Apply this diff:
- private int index = 0; + private final int index;The initialization to 0 is redundant since the constructor always sets it.
29-36: Consider validating the column index parameter.The constructor accepts any
intvalue forindexColumnFilter, including negative numbers. If an invalid column index is provided, it could lead to unexpected behavior whengetColumnText()is called in the parent class'sisLeafMatch()method.Consider adding validation:
public SpecificColumnPatternFilter(int indexColumnFilter) { + if (indexColumnFilter < 0) { + throw new IllegalArgumentException("Column index must be non-negative, got: " + indexColumnFilter); + } this.index = indexColumnFilter; }Alternatively, document the valid range in the Javadoc:
/** * Creates a new filter with the specified column index. - * @param indexColumnFilter the index of the column to apply the filter on + * @param indexColumnFilter the index of the column to apply the filter on (must be >= 0) */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF(3 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer2.java(1 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java(1 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/MultiTreePatternFilter.java(2 hunks)tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer3.java
🔇 Additional comments (7)
tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF (3)
5-5: LGTM! Bundle version bump is appropriate for new API additions.The minor version increment from 9.1.2 to 9.2.0 correctly reflects the addition of new public APIs (AbstractSelectTreeViewer3, SpecificColumnPatternFilter, and the protected getIndexColumnFilter() method).
156-156: LGTM! Import is correctly marked as optional.The
org.eclipse.tracecompass.traceeventloggerimport is used in AbstractSelectTreeViewer2 (lines 75-76) and is appropriately marked withresolution:=optionalfor optional dependency support.
34-35: Verify that new bundle dependencies are actually used.The PR adds
org.eclipse.equinox.p2.uiandorg.eclipse.ui.browserbundle dependencies to the manifest, but these are not referenced in any of the provided code changes. The PR description focuses on tree viewer filtering and does not mention P2 provisioning or browser functionality. Before merging, confirm these dependencies are required by searching the modified code for imports or usages oforg.eclipse.equinox.p2.ui.*andorg.eclipse.ui.browser.*packages. If unused, remove them to avoid unnecessary bundle bloat.tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/MultiTreePatternFilter.java (2)
77-78: LGTM! Clean refactoring to support dynamic column selection.The change from a hard-coded column index to calling
getIndexColumnFilter()is well-executed and maintains backward compatibility.
88-96: LGTM! Well-designed extension point for subclasses.The new
getIndexColumnFilter()method provides a clean template method pattern that allows subclasses to customize the filtering column while maintaining backward compatibility with a sensible default.tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/tree/AbstractSelectTreeViewer2.java (1)
223-224: LGTM! Constructor visibility change enables subclassing.Changing the constructor from
privatetoprotectedcorrectly enables AbstractSelectTreeViewer3 to extend this class. This is a safe API change since private members were not accessible to external code anyway.Note: The PR title mentions making this class "injectable," but this visibility change alone doesn't add dependency injection support—it simply enables inheritance. If dependency injection framework support (e.g., JSR-330 @Inject) was the goal, additional annotations would be needed.
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/dialogs/SpecificColumnPatternFilter.java (1)
38-45: LGTM! Clean override implementation.The method correctly overrides the parent's
getIndexColumnFilter()to return the configured column index.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF(3 hunks)
🔇 Additional comments (2)
tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF (2)
33-35: Validate newRequire-Bundleentries across all target platformsThe added/updated requirements for:
org.eclipse.jdt.annotation;bundle-version="[2.0.0,3.0.0)";resolution:=optionalorg.eclipse.equinox.p2.ui;bundle-version="2.8.500"org.eclipse.ui.browser;bundle-version="3.8.300"look fine, but please confirm:
- These bundles (with at least the specified versions) are present in all supported product configurations.
- The hard requirements on
org.eclipse.equinox.p2.uiandorg.eclipse.ui.browserare intentional (i.e., views using the new functionality will never be instantiated without them).If any usages are optional or behind feature flags, consider marking the corresponding bundles as optional as well.
156-156: Good use of optionalImport-Packagefororg.eclipse.tracecompass.traceeventloggerMarking
org.eclipse.tracecompass.traceeventloggerasresolution:=optionalis appropriate if the logger is an optional integration; it avoids wiring failures when the package is absent. Just ensure all call sites handle the package not being available (e.g., by checking class availability or using service lookups defensively).
MatthewKhouzam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Welcome to the tree!
|
|
The API can be incremented, no problem! |
|
Hey, I think you need to update the pom too! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF(1 hunks)tmf/org.eclipse.tracecompass.tmf.ui/pom.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tmf/org.eclipse.tracecompass.tmf.ui/META-INF/MANIFEST.MF
|
|
||
| <artifactId>org.eclipse.tracecompass.tmf.ui</artifactId> | ||
| <version>9.1.2-SNAPSHOT</version> | ||
| <version>9.2.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify version consistency across the tmf.ui bundle
echo "=== Checking MANIFEST.MF version in tmf.ui ==="
fd -t f "MANIFEST.MF" tmf/org.eclipse.tracecompass.tmf.ui --exec cat {} \; | grep -i "bundle-version\|implementation-version"
echo -e "\n=== Checking pom.xml in tmf.ui directory ==="
cat -n tmf/org.eclipse.tracecompass.tmf.ui/pom.xml | head -30
echo -e "\n=== Searching for other version references (9.1.2 or 9.2.0) in tmf.ui ==="
rg "9\.[12]\." tmf/org.eclipse.tracecompass.tmf.ui --type xml --type properties
echo -e "\n=== Checking feature.xml files in parent directories that might reference tmf.ui ==="
fd -t f "feature.xml" | head -20Repository: eclipse-tracecompass/org.eclipse.tracecompass
Length of output: 2844
🏁 Script executed:
#!/bin/bash
# Check feature.xml files that reference the tmf.ui bundle
echo "=== Searching feature.xml files for tmf.ui references ==="
fd -t f "feature.xml" | xargs grep -l "org.eclipse.tracecompass.tmf.ui" 2>/dev/null | while read file; do
echo -e "\n--- $file ---"
grep -n "org.eclipse.tracecompass.tmf.ui" "$file" -A 1 -B 1
done
echo -e "\n=== Checking if any feature.xml specifies bundle versions for tmf.ui ==="
fd -t f "feature.xml" | xargs grep "org.eclipse.tracecompass.tmf.ui" 2>/dev/null | grep -E "version|plugin"Repository: eclipse-tracecompass/org.eclipse.tracecompass
Length of output: 3757
Version bump is consistent across pom.xml and MANIFEST.MF.
The minor version increment from 9.1.2-SNAPSHOT to 9.2.0-SNAPSHOT appropriately reflects the new public APIs (AbstractSelectTreeViewer3, SpecificColumnPatternFilter) and visibility changes. The MANIFEST.MF Bundle-Version is correctly set to 9.2.0.qualifier, and feature.xml imports use "greaterOrEqual" constraints that are compatible with 9.2.0.
No other module POMs require version updates; the parent pom version (11.0.0-SNAPSHOT) intentionally differs from child module versions in this project structure.
Verify that Tycho CI builds successfully with this version bump.
🤖 Prompt for AI Agents
In tmf/org.eclipse.tracecompass.tmf.ui/pom.xml around line 23, the pom version
was bumped to 9.2.0-SNAPSHOT; confirm and ensure Tycho builds succeed by running
the CI/tycho build locally or via CI, verify MANIFEST.MF Bundle-Version is
9.2.0.qualifier and feature.xml imports remain compatible, and if the Tycho
build fails, adjust any module POMs or manifest entries to match the 9.2.0
versioning or fix dependency constraints so the Tycho CI passes.
@MatthewKhouzam Thanks for the reminder! The pom.xml is now updated. Please let me know if there are any additional steps or checks needed before merging. |
|
Hi @MatthewKhouzam,could you please review and approve my pull request? The workflows are awaiting maintainer approval, and merging is currently blocked. Thank you! |
|
Hi @MatthewKhouzam ! It's just a kindly reminder. Could you help me take a look at my pull request? |
What it does
AbstractSelectTreeViewer2injectable to improve flexibility and integration with dependency injection frameworks.AbstractSelectTreeViewer3, which extends the tree viewer functionality to support column-aware filtering usingSpecificColumnPatternFilter.MultiTreePatternFilter2to allow filtering based on a specific column index, enhancing usability for multi-column trees.How to test
AbstractSelectTreeViewer3or configure a tree viewer withSpecificColumnPatternFilter./and verify that filtering works correctly for the specified column index.AbstractSelectTreeViewer2without breaking existing functionality.Follow-ups
Review checklist
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.