Feature/develop2 info package filters #12836
Conversation
There was a problem hiding this comment.
In the original issue (Or elsewhere) you mentioned that it made little sense to add filtering to outputs meant for further machine processing because you can always filter the resulting json by yourself. I'm guessing that by implementing this, we're walking back on that, right?
conan/cli/formatters/graph/graph.py
Outdated
| serial = graph.serialize() | ||
| # TODO: This is not used, it is necessary to update the renderings to use the serialized graph | ||
| # instead of the native graph | ||
| serial = filter_graph(serial, package_filter, field_filter) |
There was a problem hiding this comment.
Do we need to save the output of the function if it's not used elsewhere?
There was a problem hiding this comment.
It is temporary, with the TODO. The idea is that this serial is the thing that should be fed to the rendering templates, and not the graph object.
There was a problem hiding this comment.
Ohh now the PR comment makes sense, I didn't quite understand what it was in reference to, ty for the clarification!
Yes, we were reconsidering this, and apparently it would match user expectations, if they provide both the json and a filter is because they want it filtered. It also makes sense to be able to trim the output html/dot graph to the packages that you want, so the filter seems also useful for the other formatters. That seems to mean that all the formatters should be consistent with the user input, filtering out whatever the user wants to filter out. Wdyt? |
|
Yes, that's how I would have guessed all formatters behaved had I not seen the issues related to this, so I think that this change is very useful for those cases :) |
conan/cli/formatters/graph/graph.py
Outdated
| @@ -99,6 +100,14 @@ def _render_graph(graph, template, template_folder): | |||
| def format_graph_html(result): | |||
| graph = result["graph"] | |||
| conan_api = result["conan_api"] | |||
| field_filter = result["field_filter"] | |||
| if field_filter and "requires" not in field_filter: | |||
There was a problem hiding this comment.
should it check for field_filter is not None instead (as empty list may evaluate to False in python)?
There was a problem hiding this comment.
Removing this, it doesn't really apply
Changelog: Fix: Make
graph infofilters to work on json output tooDocs: Omit
Close #12533
There is still some larger work pending in this area, but too much for this PR: