Show nested projects co-located and indented - #9602
Conversation
|
Awesome! |
mbien
added
Platform
… are reindented due to open/close
mbien
left a comment
mbien
left a comment
There was a problem hiding this comment.
I think this has potential. Since this is solved via UI, it avoids many issues e.g having to think about how to represent open/closed projects in trees etc - it builds on top of the existing logic, project groups etc.
It does also automatically work for gradle too:
Visually it will likely need some tweaks since it moves the expand/collapse control away from the icon (visible only deeper in the tree). Comparison (right is NB 31):
indentation amount could also become a UI property so that it can be easier themed - but this could be done in followups.
| var node = Visualizer.findNode(vis); | ||
| while (node != null) { | ||
| if (node instanceof ProjectsRootNode.BadgingNode) { | ||
| var badge = (ProjectsRootNode.BadgingNode) node; | ||
| return badge.pair.depth; |
There was a problem hiding this comment.
nitpick: we could bump the language level to 21 to use the instanceof pattern.
if (node instanceof ProjectsRootNode.BadgingNode badge) {
return badge.pair.depth;
}pet peeve: I am personally not a fan of using var everywhere since it obfuscates code and is in many cases not necessary when the type is already concise. But I am not going to argue against it, its more about code style and mostly non technical.
Its just a bit unfortunate that the trend of overusing var made IDEs render the type next to it which entirely defeated the purpose of it in the first place since it now uses more space than before and breaks formatting:
but that is just my "pet peeve" - feel free to ignore it ;)
There was a problem hiding this comment.
- yes, I wanted to use
instanceofpattern too - I've noticed the NetBeans IDE support of
varis pretty poor- for example move refactoring replaces
varwith FQN types!?- I hope I fix it one day.
- probably a result of this kind of attitude towards
var
- for example move refactoring replaces
- I don't share such attitude
- For last for years I was coding in languages where it is enough to write
x = 3.14 - even
varis too verbose for my taste
- For last for years I was coding in languages where it is enough to write
- I prefer use of
varin my code over needless verbosity of specifying the type at each assignment
There was a problem hiding this comment.
I've noticed the NetBeans IDE support of var is pretty poor (...) or example move refactoring (...) probably a result of this kind of attitude towards var
facepalm. Not sure what to say to statements like this.
For last for years I was coding in languages where it is enough to write x = 3.14
DSLs are great for their domains. That is why there are so many languages.
But it is OK to disagree. As I said I am not going to block anything based on style.
There was a problem hiding this comment.
Yes, we've had lots of fixes for var, and another one just one my radar to look at. We did talk about blocking use of var at one point in style guidelines for the IDE code, but I think we ended up with a general feeling of making sure it's legible. Personally I think that's a judgement call based on making sure type information is easily understandable from reading text in the vicinity without requiring IDE annotations - eg. assignments with new may be OK whereas assignments from method calls may not.
| if (list.getModel() instanceof NodeListModel && (((NodeListModel) list.getModel()).getDepth() > 1)) { | ||
| int indent = iconWidth * NodeListModel.findVisualizerDepth(list.getModel(), vis); | ||
| int indent = iconWidth * findIndent(list.getModel(), vis); |
There was a problem hiding this comment.
nit: instanceof pattern would clean this up too
| if (model instanceof ListModel) { | ||
| return NodeListModel.findVisualizerDepth((ListModel) model, visualizer); |
| if (p2 == null) { | ||
| return 1; | ||
| } | ||
| return p1.getProjectDirectory().getPath().compareTo(p2.getProjectDirectory().getPath()); |
There was a problem hiding this comment.
Alphabetical String comparison with paths seems to be indeed working well for this. My instinct told me that this will probably break with edge cases but it is always making sure that parent dir ends up before child dir. This is good for this purpose (I think) and also fast.
Path comparison has the limitation that the projects do actually have to be a tree structure on disk since it shows the filesystem hierarchy, not the actual project parent which could be anywhere. It correlates location with relationship between projects. This might be fine still - curious what others think.
There was a problem hiding this comment.
projects do actually have to be a tree structure on disk
- I don't think there is much to improve without enhancing Project API.
LogicalViewProvider.WithNestedProjectsallows one to logically nest projects #9592 tried the enhance the API route- and we decided we rather keep it simple
- hence I'd rather stick with tree structure on disk for purposes of this PR
There was a problem hiding this comment.
and we decided we rather keep it simple
Decided?? I think both approaches have pros and cons right now. This simpler approach might be the better one, and the least intrusive, but having the view not end up reflecting the underlying model might end up causing issues down the line.
+1
This is my concern too, but it's more than just "visual" in the sense that it affects also where you can click to control expansion. We'll end up with a tree UI that diverges more and more from the underlying model. That might start to get "interesting". The other UI thought that comes to mind with this is the potential to change the root of the project tree to just one project / subproject, with a breadcrumb bar (similar to eg. the GNOME file browser in list mode when tree expansion is enabled). |
5 participants
NodeRendererand control its (already existing and used inChoiceView) indentation support