tfausak / cabal-gild

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

Exclude option must be interpreted relative to cabal file #71

Closed konn closed 4 months ago

konn commented 4 months ago

I'm using cabal-gild on monorepo, using find command combined with cabal-gild --io to format everything. For now, cabal-gild shipped with HLS is slightly older and does not support exclusion mechanism, so I use the following command on every save:

find . -name 'cabal.project' -or -name 'cabal.project.local' -or -name '*.cabal' -not -path './dist-newstyle/*' | \
  while read -r FILE; do cabal-gild --io "${FILE}"; done

This work as expected as long as --exclude isn't used in cabal files. OTOH, if one uses relative path in --exclude, then cabal-gild resolves the path relative to current working directory, not relative to *.cabal files.

For example, suppose we have project-core/project-core.cabal with the following contents:

...
test-suite project-core-test
  import: defaults
  type: exitcode-stdio-1.0
  main-is: Test.hs
  -- cabal-gild: discover test --exclude=test/Test.hs
  other-modules:
  hs-source-dirs: test

If one invoke cabal-gild inside project-a directory, it just emits an empty other-modules. On the other hand, if one uses the above find-script in the monorepo root, it emits:

test-suite project-core-test
  import: defaults
  type: exitcode-stdio-1.0
  main-is: Test.hs
  -- cabal-gild: discover test --exclude=test/Test.hs
  other-modules: Test

If one changes the exclude relative to monorepo root, it works as expected:

test-suite project-core-test
  import: defaults
  type: exitcode-stdio-1.0
  main-is: Test.hs
  -- cabal-gild: discover test --exclude=project-a/test/Test.hs
  other-modules:

As cabal resolves relative paths according to *.cabal file, I think it would make sense to paths to be resolved relative to cabal files.

tfausak commented 4 months ago

Oof yeah, that's a bug. The excluded files should be relative to the Cabal file that they're referenced in. This line will have to change to incorporate the root:

https://github.com/tfausak/cabal-gild/blob/51425c05c67e6c335ba9024ced779669e3a4aca0/source/library/CabalGild/Unstable/Action/EvaluatePragmas.hs#L81

This wasn't caught by the tests because all the ones for the --exclude flag use STDIN, so the Cabal file's directory is the same as the CWD. Another test should be added like this one:

https://github.com/tfausak/cabal-gild/blob/51425c05c67e6c335ba9024ced779669e3a4aca0/source/test-suite/Main.hs#L66

I'd be happy to accept a PR to fix this issue. It may take me a while to get around to it.

fendor commented 4 months ago

For now, cabal-gild shipped with HLS is slightly older and does not support exclusion mechanism

If I am not mistaken, HLS doesn't ship with cabal-gild and uses the binary on the $PATH. Only the tests pull in cabal-gild in HLS. So, perhaps this is another issue.

tfausak commented 4 months ago

Maybe HLS runs the cabal-gild command from some other directory, perhaps even a temporary one?

fendor commented 4 months ago

Should be the directory of the .cabal file: https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-cabal-gild-plugin/src/Ide/Plugin/CabalGild.hs#L77

konn commented 4 months ago

For now, cabal-gild shipped with HLS is slightly older and does not support exclusion mechanism

If I am not mistaken, HLS doesn't ship with cabal-gild and uses the binary on the $PATH. Only the tests pull in cabal-gild in HLS. So, perhaps this is another issue.

Ah, it seems that I use the current released version of Haskell extension for VSCode and it does not support haskell.cabalFormattingProvider item, which makes HLS of course ignores cabal-gild pragma as it uses cabal-fmt as a formatter! So please ignore the claim about the HLS.

konn commented 4 months ago

@tfausak I've just filed PR to fix this issue: #73