-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] Add a Tree Helper + multiple Styles #59588
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 8000 terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
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.
I like it!
Memory consumption should be considered as this deals with potentially large directories, ideally nodes should be yielded as they come.
Regarding the public API, we should remove or internalize any getter/setter for which we don't have a strong use case.
Yep! I'll make the changes depending on the following questions.
Do you have some examples in mind ? Do we need both |
I was refering to |
|
I've removed the getters and added a "appy" method on TreeStyle (we can consider public function applyPrefixes(RecursiveTreeIterator $iterator): void
{
$iterator->setPrefixPart(RecursiveTreeIterator::PREFIX_LEFT, $this->prefixLeft);
$iterator->setPrefixPart(RecursiveTreeIterator::PREFIX_MID_HAS_NEXT, $this->prefixMidHasNext);
$iterator->setPrefixPart(RecursiveTreeIterator::PREFIX_MID_LAST, $this->prefixMidLast);
$iterator->setPrefixPart(RecursiveTreeIterator::PREFIX_END_HAS_NEXT, $this->prefixEndHasNext);
$iterator->setPrefixPart(RecursiveTreeIterator::PREFIX_END_LAST, $this->prefixEndLast);
$iterator->setPrefixPart(RecursiveTreeIterator::PREFIX_RIGHT, $this->prefixRight);
}Also removed getStyle / setStyle Will refactor as lazy iterator but i'm not sure how to handle the cycles yet |
|
@smnandre It will be really useful 🙏 great job |
e4e66de to
5ffb3fd
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.
💯
|
I've fusioned Removed the Finder integration for now (i'd rather have separated PR as it will require some discussion) |
|
Open to review :) |
| @@ -369,6 +373,24 @@ private function getProgressBar(): ProgressBar | |||
| ?? throw new RuntimeException('The ProgressBar is not started.'); | |||
| } | |||
|
|
|||
| /** | |||
| * @param iterable<string, iterable|string|TreeNode> $nodes | |||
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.
Some words or example about what can be passed would be nice either here or in TreeHelper or both
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.
HA ! It is to avoid back and forth (with a future link to the doc) :), a sample like :
$treeHelper = new TreeHelper($output, TreeNode::fromValues([
'a' => [
'a1',
'a2'
],
'b',
'c' => [
'c1',
'c2' => [
'c21' => [
'c211',
'c212',
],
],
],
],
new TreeNode($rootNode),
), TreeStyle::rounded());
$treeHelper->render();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.
A few comments but looks good otherwise 🙂
| $parent->addChild($this); | ||
| } | ||
|
|
||
| foreach ($children as $child) { |
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.
iterating in the constructor breaks lazy evaluation of the iterable, is that justified?
|
|
||
| public static function box(): self | ||
| { | ||
| return new self( |
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.
on one line things would be better visually - not consuming my screen's space for nothing ;)
|
Last comments are none blocking suggestion |
177f738 to
6a641c9
Compare
|
Thank you @smnandre. |
This PR was squashed before being merged into the 7.3 branch. Discussion ---------- [Console] Document the `TreeHelper` | Q | A | | - | - | | Feature PR | [symfony/symfony#59588](symfony/symfony#59588) | | PR author(s) | [`@smnandre`](https://github.com/smnandre) | | Merged in | 7.3 | | Doc Issue | Fix #20692 | fix #20692 Commits ------- d9a48be [Console] Document the `TreeHelper`
Important
Code examples below are not up to date, documentation incoming.
Feature
The goal of the Tree helper is to ease rendering hierarchical data in the console:
Usage
Two methods available, manual or via SymfonyStyle
SymfonyStyle (easiest)
Will render
You also can pass a custom TreeStyle here, or one of the one availbes (see below)
Manually
You can build the Tree the way you want thanks to the TreeNode class.
Styles
Various styles are available (based on the styles of Table .. and some new ones), or you can also build your own Style from scratch.
Default
Box
Box Double
Rounded
Compact
Light
Minimal