tfausak / cabal-gild

:crown: Format Haskell package descriptions.
https://hackage.haskell.org/package/cabal-gild
MIT License
44 stars 4 forks source link

Duplicated command line arguments are ignored #80

Closed TristanCacqueray closed 1 day ago

TristanCacqueray commented 2 weeks ago

Hello, thank you for cabal-gild, it's working great!

Though I had a little usability issue when using it to format multiple cabal files. I thought cabal-gild would work like fourmolu where you can give it multiple files at once. However, it seems like it silently ignores duplicated arguments and it only process the last one.

For example, this invocation:

$ cabal-gild --io ./lib/lib.cabal --io ./server/server.cabal --io ./client/client.cabal

… only format the client.cabal file. The other ones are not touched.

Would it makes sense to support this use case, or at least throw an error in that case?

tfausak commented 2 weeks ago

I think this is similar to #35.

Everything's working as I expect it to: later command line options override earlier ones. However clearly that's confusing in this case.

I'd suggest using a shell script for this, like: sh -c 'for f in */*.cabal; do cabal-gild --io $f; done'.

tfausak commented 2 weeks ago

In general I think it would be good to warn when the same flag is passed multiple times.

For the --io flag specifically, Gild could allow it to be passed multiple times and interpret it as a list. That way your usage would work.

I didn't implement it like that originally in response to #35 because you'd still need some kind of shell scripting to make it work. And if shell scripts are involved, cabal-gild --io $x; cabal-gild --io $y isn't much different than cabal-gild --io $x --io $y.

Aside from accepting multiple --io flags, I think the approach outlined in this comment is still the most reasonable way to support formatting multiple files: https://github.com/tfausak/cabal-gild/issues/35#issuecomment-1969536376

TristanCacqueray commented 1 week ago

Alright, thanks for the explanation. Using multiple invocations fixed my issue. Should this be tagged with good-first-issue?

tfausak commented 1 week ago

For improving the ergonomics here, I think the path forward looks like this: Update Config.applyFlag to accumulate a collection of warnings. Then in Context.fromConfig, emit those warnings. Arguably Flag.fromArguments should be updated too, to emit warnings rather than errors for UnexpectedArgument and UnknownOption.