My primary motivation is making the `mvn dependency:tree -DoutputType=dot` output usable. That's why I've been looking at that code - I've used 'dependency' for years.
I can definitely look at repeating the effort but unless the existing implementation is going away soon I would prefer to finish my current work and then circle back to this. (This is actually about 3 levels deep from my primary focus - it's been a series of "oh, if I have *this* then I can do *that* more quickly. I would like to get back to those other tasks asap.) I'm having trouble creating a PR so I can point you to what I have as a start. (I suspect it's because I added an 'examples' directory so I can include links instead of attachments.) The fork is at https://github.com:/coyotesong/maven-dependency-plugin. This directory contains the results of running 'mvn dependency:tree' on this project. https://github.com/coyotesong/maven-dependency-plugin/tree/master/examples. The generated image files are located in https://github.com/coyotesong/maven-dependency-plugin/tree/master/examples/dot/3.8.2-SNAPSHOT . The proposed changes are pretty much all in this package: https://github.com/coyotesong/maven-dependency-plugin/tree/add-velocity-utils/src/main/java/org/apache/maven/plugins/dependency/utils/templates There's also the test class, some test templates, etc. Bear On Sun, Mar 30, 2025 at 1:57 PM Guillaume Nodet <gno...@apache.org> wrote: > Which project are you targeting exactly ? > If you're targeting Maven Core, do you think you could target the new API > rather than the resolver one ? > > > https://github.com/apache/maven/blob/master/api/maven-api-core/src/main/java/org/apache/maven/api/Node.java > Or are you targeting the resolver ? > > Le dim. 30 mars 2025 à 19:04, Bear Giles <bgi...@coyotesong.com> a écrit : > > > I've taken the time to look around the other 'tree' classes, or more > > precisely the '*DependencyNodeVisitor" classes, and noticed that most if > > not all of them follow the anti-pattern of doing more than one thing in > > each method. More precisely they mix logic ('what to include') and > > presentation ('what to send to the writer). This makes code difficult to > > understand, to test, to extend by a third party, etc. > > > > This is why template engines were developed! :-) > > > > Fortunately there's already a dependency on 'velocity', although it > appears > > to only be used during the extensive testing. However it's still there in > > the 'compile' scope so using it won't require adding new dependencies. > > > > That's a very clean solution to the problem identified above. For most > > formats the velocity template won't need anything beyond what's already > > included in the WrappedNode (DependencyNode). If a format can benefit > from > > adding a tiny bit of additional information it can be added to the > > WrappedNode since that class is exclusive to the 'tree' package. In more > > complex cases the individual class can maintain it's own data structures > > and add them to the Context provided to velocity. > > > > This should work with all of the existing classes although I'm only > > planning to modify one at this time. It should make the classes much > easier > > to maintain since they can focus on the logic of how to convert the data > > provided by the DependencyNode to the data usable by the Velocity > engine. I > > know other template engines, e.g., jinjava, have a way to provide > > user-defined functions but I don't think velocity does - that's > unfortunate > > since it will require a modest amount of pre-computation. Fortunately > that > > can be handled separately, immediately before calling the Velocity > 'merge' > > method. > > > > This will only require a few classes - basically just a wrapper to set up > > the velocity engine. It will also require a standard place to put the > > templates and I recommend 'templates/{format}/{name} and > > 'templates/{format}/macros/{name}. The 'format' would be 'dot', 'json', > > etc., and the 'macros' allows velocity macros to be kept separately from > > the templates since they're much more likely to be reused. (Hmm... > perhaps > > also 'templates/macros/' for macros that are format-agnostic. > > > > ----------------------------- > > > > The second PR will probably be a bit more controversial since there's > > multiple ways to implement it and comments in some of the 'upstream' > > methods suggest there's already WIP that may affect how the data is > > provided to the *DependencyNodeVisitor classes. > > > > At the moment these classes only include two methods - visit(node) and > > endVisit(node). It is the responsibility of the endVisit(node) method to > > determine that all data has been received. I think that's an obvious > > omission but the DependencyNodeVisitor interface is defined in a > different > > project. It would be trivial to add to the existing implementations, and > > java8 supports optional methods so there would be no impact on others if > > this method is added to the interface with an empty optional method. > > > > That's not a big problem if your visitor is able to run in a 'streaming' > > mode, but some formats can benefit from using a 'document' mode instead. > > In most cases this doesn't matter since the results are not intended for > > human consumption - they need to be a standard format accepted by other > > applications and these formats are usually designed to work in a > streaming > > fashion. > > > > However the dot format, and potentially other, are intended for human > > consumption. The file itself isn't human-friendly but there are widely > > available tools to convert it into human-friendly images and even > > interactive formats like svg. In these cases it makes much more sense to > > use a document mode when creating the final document since it is often > > appropriate to provide the same information, or at least related > > information, in multiple places in the document. > > > > So there's clearly a benefit to a new method that is called immediately > > after the final endVisit(), one that provides the complete dependency > tree. > > The implementations should be able to ignore visit() and endVisit(). (And > > in fact they can be added to the abstract class if not the > > DependencyNodeVisitor interface) > > > > The question is how to implement this, esp. when looking at other > potential > > enhancements to the class. > > > > For instance it's possible to have DependencyNodeVisitor implement > > Consumer<DependencyNode> and the 'accept()' method calls visit(). It may > > not be practical since many implementations will need the nesting that > > endVisit() provides. > > > > Another possibility is to have DependencyNodeVisitor implement > > Function<DependencyNode, Boolean> that's called after the final > endVisit() > > call. Again the idea is to simplify integration into streams but there's > no > > reason you couldn't just use the (object::function) notation. > > > > I think this ultimately comes down to what's best with future > improvements > > to the upstream code that creates the data sent to these classes. There > are > > notes that indicate the desire to shift to a streaming pattern (while > > maintaining legacy interfaces, of course) and it would make sense to use > a > > consistent approach here - even if it means reformatting the data > > immediately before calling the new method. > > > > For now I'll just add a new method to the abstract class that all of the > > implementations extend but I consider it a stopgap measure since this > > functionality would be useful to third party developers, etc. > > > > (Sidenote - it could even quietly replace some of the existing logic > since > > the final method could easily perform a recursive descent of rootNode and > > call visit() and endVisit() itself. I think this could be put into the > > DependencyNodeVisitor interface as a default method so there would be no > > risk to removing the existing code. The primary drawback would be that > the > > current approach allows the implementation to bail out early if it hits a > > problem while this approach would require the entire rootNode structure > to > > be generated first.) > > > > Bear > > > > > -- > ------------------------ > Guillaume Nodet >
