Add crystal tool dependencies
#13613
Conversation
| config = create_compiler "tool dependencies", no_codegen: true | ||
| config.compiler.no_codegen = true | ||
|
|
||
| config.compiler.dependency_printer = dependency_printer || DependencyPrinter.new(STDOUT) | ||
| config.compile |
There was a problem hiding this comment.
Can this be expressed as an extra parameter of Crystal::Command#compile_no_codegen? In that case the command-line argument handling would be done inside #create_compiler itself, similar to hierarchy
There was a problem hiding this comment.
What would be the benefit? It seems to me this is a pretty simple setup. Yes, it's some duplicate code. But that's not bad. Combining everthing in one place adds quite a bit of complexity.
There was a problem hiding this comment.
I haven't run this branch yet, but I believe this means the tool doesn't have access to any of the common command-line options that other tools have (-D, --prelude etc.)
There was a problem hiding this comment.
There is indeed a problem with this approach: crystal tool dependencies still shows the full help rather than this help, but the option parser only ever accepts this -f, and not the actually shown options like -h. In particular this -f tree|flat conflicts with the other -f text|json.
There was a problem hiding this comment.
If crystal tool hierarchy accepts -D and --prelude, so should this tool; this is not merely a matter of code duplication
There was a problem hiding this comment.
The actual common piece that should be reused for a single option parser instance is create_compiler. Utilizing compile_no_codegen would add unnecessary complexity for no real gain.
Co-authored-by: Quinton Miller <nicetas.c@gmail.com>
|
how hard would it be to remove a branch? like |
|
@beta-ziliani It won't be hard. But I would prefer to keep this PR focused on a minimal implementation. Let's talk about these enhancements in a separate issue. |
|
@straight-shoota Just as a suggestion, if it's possible to add redundant dependency warnings in the Crystal file that would be great. |
|
PR title still refers to the singular name (dependency vs dependencies) |
beta-ziliani
changed the title
crystal tool dependencycrystal tool dependencies
|
I've changed the target branch to a feature branch on this repo (#13631). I'll be making a couple of individual PRs to that branch and we can merge that all together into master after the freeze period for 1.9 is over. |
This tool prints all source files read by the compiler in their require order.
It's a basic implementation with two format options,
treeandflat. They are identical except thatflathas no indent.Files are printed exactly in the order they are required. Duplicates (i.e. when a file had already been required before) are not shown.
Example output for an empty file
Resolves #11481