jbang
jbang copied to clipboard
fix: Unable to use {basename} in template file-refs
- {filename} and {basename} have different validation rules for their usage
- fixes #1318
Codecov Report
Merging #1319 (84965d1) into main (e61fcb4) will decrease coverage by
0.01%. The diff coverage is100.00%.
:exclamation: Current head 84965d1 differs from pull request most recent head 55fab25. Consider uploading reports for the commit 55fab25 to get more accurate results
@@ Coverage Diff @@
## main #1319 +/- ##
============================================
- Coverage 56.64% 56.62% -0.02%
+ Complexity 1154 1153 -1
============================================
Files 99 99
Lines 6128 6128
Branches 1019 1019
============================================
- Hits 3471 3470 -1
Misses 2176 2176
- Partials 481 482 +1
| Flag | Coverage Δ | |
|---|---|---|
| Linux | 55.43% <100.00%> (-0.02%) |
:arrow_down: |
| Windows | 56.05% <100.00%> (-0.02%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/main/java/dev/jbang/cli/Init.java | 84.76% <100.00%> (-0.96%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update e61fcb4...55fab25. Read the comment docs.
Can I ask for a review @quintesse @maxandersen ?
I added an extra test that shows where your code causes problems. That test should already have existed of course but it didn't :-)
So it's not that we disagree with what you want to do, it's a very useful feature. But the way templates work is honestly a bit obscure. So right now a template needs at least one of these two things to be true:
- there is at least a single entry named
{filename}=somefile.ext[.qute]and the name given by user to theinitcommand should have the same.extextension as the file after the=sign - there is at least a single entry named
{basename}.ext=somefile.someext[.qute]and the name given by user to theinitcommand should have the same.extextension as the file before the=sign
Right now JBang does this check for all files in a template, which is not really what you want because it prevents a template author from doing exactly what you want to do.
But we still need to make sure that at least one file follows one of these rules!
And right now your code only follows the first rule.
The code would have to be adapted in such a way that it checks that the first file it encounters that matches those rules is enough. So having:
{filename}=somefile.java
{basename}.md=somefile.md
is correct, but so is:
{basename}.java=somefile.java
{basename}.md=somefile.md
And in both cases JBang has to check that typing init myfile.java is correct and so is init myfile, but typing init myfile.foo or init myfile.md should throw an error.
Edit: so to make sure that this is clear: in reality a template should either only contain a single {filename} entry (and an arbitrary number of {basename} entries), or it should contain only {basename} entries in which case the first one is the one that will get the name the user typed on the command line.
Perhaps the best thing to do would be to do is to split this code into two steps: https://github.com/jbangdev/jbang/blob/main/src/main/java/dev/jbang/cli/Init.java#L67-L76
The first step checks if the correct rules are followed and throws an error when it finds a problem. And then the second step would be the current code without the error checking.
Wow.
Thanks for the input, let me dig deeper and find a good solution for this as soon as I have time.