Document @file CLI arguments

[grodin]

Parsing args from a file if the first argument is @filename was introduced in this commit but doesn't seem to be documented anywhere apart from a cryptic hint in cli help. I only found it because I got curious so went digging in the source, but it's actually a useful feature which deserves documenting!

[hick209]

@grodin, that's a common practice for CLIs and often CLI tools offer that right out of the box, but sure we could document that.

Do you want to submit a PR for it?

[grodin]

Can do. Should have some time next week.

[grodin]

Starting work on this, I'd forgotten that the CLI is pretty barebones (e.g. lacks --help flag, only shows generic usage when failing to parse command line). Would you be interested in me improving the UX of the CLI a bit (adding --help mainly)?

Or should I just document the @argfile feature minimally, in the CLI and on the website?

Happy to do either, but might be overstepping the mark a bit with the first if you're wanting to keep the CLI ultra-minimal.

[hick209]

I'd be happy to have it improved! 😃

Just maybe make it into different PRs, if possible

[grodin]

Yep, no problem

[grodin]

How far can I go in rebuilding the CLI?

One option is to go the whole hog and use clikt but obviously that's a new dependency and possibly bloating a pretty small Jar.

The current CLI behaviour is encoded in tests. Should I try to keep those tests passing or can I replace them?

[hick209]

Although I love clikt and use it in other projects, we intentionally didn't use it (or other cli libs) to keep the library slim.

Feel free to go as deep as you want on refactoring.
Once you have something, even if not really, share it so we can start the discussion in case we want to change somethings up

[hick209]

Oh, just try not to change the CLI public API, unless it's really necessary. We're going with a 1.0 release, so that could warrant an API change.

[grodin]

Oh, just try not to change the CLI public API, unless it's really necessary. We're going with a 1.0 release, so that could warrant an API change.

Some more clarity about this would be useful. The current implementation has a fair bit of unexpected behaviour: I reported #465 separately, but the other thing is that the style flags aren't mutually exclusive, e.g.

$ ktfmt --dropbox-style --set-exit-if-changed --kotlinlang-style --google-style SomeFile.kt

is valid. The last style option on the command line wins, so --google-style in this case.

I find this fairly surprising, but I'm not certain how other CLI tools handle it. I think Clikt has the same behaviour by default, but it's not completely clear from their docs.

[hick209]

Oh, just try not to change the CLI public API, unless it's really necessary. We're going with a 1.0 release, so that could warrant an API change.

Some more clarity about this would be useful.

Basically I meant that we should try not to change the options/params of the program.
You brought up a very good example here, with the formatting styles.
Assuming that one is changed to the following:

$ ktfmt --style=dropbox

This is something I would consider a change in the CLI API.

Honestly this looks like a good change, IMHO and we could put that in the 1.0.0 release.

Regarding how to handle multiple params, from my experience, it seems common for CLIs to just take the last params specified. The Kotlin compiler, for example, does that, only issuing a warning stating that multiple values were defined and it will take the last one.
In other words, I'm fine with keeping things as they are, but if you want to improve things, you could do the same approach as the Kotlin compiler, of adding a warning message when overriding options are passed in as parameters.

[grodin]

Are you still wanting the --style= change before 1.0? It seems like it's a nice little thing to have but definitely not a blocker.

[hick209]

You can put that up as a separated PR

[hick209]

To be clear, this is a change that breaks the CLI API, so it will be part of 1.0.0.

Once this PR here lands, I'll make a v0.51 release and then could immediately merge the changes for --style= since that would only get shipped on the next release, which I plan to be 1.0.0