-
Notifications
You must be signed in to change notification settings - Fork 27
Add entry-tree parameter to collapse children #277
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
Add entry-tree parameter to collapse children #277
Conversation
| boolean isComplete = true; | ||
| List<Entry<M, Object>> entries = new ArrayList<>(); | ||
| List<ITableColumnDescriptor> columnDescriptor = null; | ||
| boolean collapseChildren = false; |
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.
This would collapse the children of the root. What about have a expandLevel parameter instead, that indicates to which level the tree should be expanded? If expandLevel is missing or has value of -1 it would mean that all the levels are expanded (as it is the current behaviour). If the expandLevel is set to 1 for example, it will expand to the first nodes under the root. WDYT?
bhufmann
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.
Thanks for the contribution. I have some comments. Please add a test case to TmfTreeModelTest (see method testTmfTreeModelConstructor2.
| * would mean that all the levels are expanded (default behaviour). If the expandLevel is set to 1 for | ||
| * example, it will expand to the first nodes under the root. | ||
| */ | ||
| private int fExpandLevel = -1; |
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.
There should be a constant available for -1 and used in the both classes.
Add this to this class:
/**
* Expand level to indicate that all tree levels are expanded. (default)
* @since 9.6
*/
public static final int ALL_LEVELS = -1;Also, rename variable to fAutoExpandLevel
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.
Done
| * Indicates which level of the tree should be expanded. If expandLevel is missing or has value of -1 it | ||
| * would mean that all the levels are expanded (default behaviour). If the expandLevel is set to 1 for | ||
| * example, it will expand to the first nodes under the root. | ||
| */ |
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.
Change the comment to:
/**
* Indicates which level of the tree should be expanded.
*/The more detail information
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.
Done
| /** | ||
| * @return | ||
| */ | ||
| public int getExpandLevel() { |
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.
rename getAutoExpandLevel
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.
Done
| /** | ||
| * @param expandLevel | ||
| */ | ||
| public void setExpandLevel(int expandLevel) { |
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.
rename setAutoExpandLevel
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.
Done
| } | ||
|
|
||
| /** | ||
| * @param expandLevel |
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.
change java doc
/**
* Sets the auto-expand level to be used for the input of the tree. The
* value 0 means that there is no auto-expand; 1 means that top-level
* elements are expanded, but not their children; 2 means that top-level
* elements are expanded, and their children, but not grand-children; and so
* on.
* <p>
* The value {@link #ALL_LEVELS} means that all subtrees should be expanded.
* </p>
* @since 9.6
*/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.
Done
| } | ||
|
|
||
| /** | ||
| * @return |
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.
change java doc
/**
* Returns the auto-expand level.
*
* @return non-negative level, or <code>ALL_LEVELS</code> if all levels of
* the tree are expanded automatically
* @see #setAutoExpandLevel
* @since 9.6There 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.
Done
| private List<ITableColumnDescriptor> fColumnDescriptors = new ArrayList<>(); | ||
| private List<T> fEntries; | ||
| private @Nullable String fScope; | ||
| private int fExpandLevel = -1; |
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.
Use constant
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.
Done
| * | ||
| * @param expandLevel | ||
| * expand level of the tree model | ||
| * @return this {@link Builder} object |
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.
add @since 9.6
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.
Done
| * expand level of the tree model | ||
| * @return this {@link Builder} object | ||
| */ | ||
| public Builder<T> setExpandLevel(int expandLevel) { |
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.
rename to setAutoExpandLevel
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.
Done
| boolean isComplete = true; | ||
| List<Entry<M, Object>> entries = new ArrayList<>(); | ||
| List<ITableColumnDescriptor> columnDescriptor = null; | ||
| int expandLevel = -1; |
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.
use constant ALL_LEVEL (see comment below for TmfTreeModel)
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.
Done
|
I'm going to update this PR with my suggestions to help with this contribution. |
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.
In noticed that the PR is based on an old commit and not on the latest in master. I'll rebase and squash the commits after approval. Since I'm continuing with this PR I'll force push the updated version then.
| boolean isComplete = true; | ||
| List<Entry<M, Object>> entries = new ArrayList<>(); | ||
| List<ITableColumnDescriptor> columnDescriptor = null; | ||
| int expandLevel = -1; |
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.
Done
| * Indicates which level of the tree should be expanded. If expandLevel is missing or has value of -1 it | ||
| * would mean that all the levels are expanded (default behaviour). If the expandLevel is set to 1 for | ||
| * example, it will expand to the first nodes under the root. | ||
| */ |
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.
Done
| * would mean that all the levels are expanded (default behaviour). If the expandLevel is set to 1 for | ||
| * example, it will expand to the first nodes under the root. | ||
| */ | ||
| private int fExpandLevel = -1; |
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.
Done
| } | ||
|
|
||
| /** | ||
| * @return |
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.
Done
| /** | ||
| * @return | ||
| */ | ||
| public int getExpandLevel() { |
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.
Done
| } | ||
|
|
||
| /** | ||
| * @param expandLevel |
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.
Done
| /** | ||
| * @param expandLevel | ||
| */ | ||
| public void setExpandLevel(int expandLevel) { |
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.
Done
| private List<ITableColumnDescriptor> fColumnDescriptors = new ArrayList<>(); | ||
| private List<T> fEntries; | ||
| private @Nullable String fScope; | ||
| private int fExpandLevel = -1; |
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.
Done
| * | ||
| * @param expandLevel | ||
| * expand level of the tree model | ||
| * @return this {@link Builder} object |
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.
Done
| * expand level of the tree model | ||
| * @return this {@link Builder} object | ||
| */ | ||
| public Builder<T> setExpandLevel(int expandLevel) { |
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.
Done
Signed-off-by: Bernd Hufmann <[email protected]>
The auto-expand level is supposed to be used for the input of the tree. The value 0 means that there is no auto-expand; 1 means that top-level elements are expanded, but not their children; 2 means that top-level elements are expanded, and their children, but not grand-children; and so on. The value -1 means that all subtrees should be expanded (default). [Added] Add auto-expand level in TmfTreeModel Signed-off-by: Hriday Panchasara [email protected] Signed-off-by: Bernd Hufmann <[email protected]>
03300f2 to
9f48c7b
Compare
What it does
tmf: Add auto-expand level in TmfTreeModel
The auto-expand level is supposed to be used for the input of the tree. The value 0 means that there is no auto-expand; 1 means that top-level elements are expanded, but not their children; 2 means that top-level elements are expanded, and their children, but not grand-children; and so on.
The value -1 means that all subtrees should be expanded (default).
Contributes to fix eclipse-cdt-cloud/theia-trace-extension#801
How to test
Run updated unit test.
Follow-ups
Related incubator PR.
eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#196
Review checklist
Signed-off-by: Hriday Panchasara [email protected]
Signed-off-by: Bernd Hufmann [email protected]