Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude option must be interpreted relative to cabal file #71

Closed
konn opened this issue May 14, 2024 · 6 comments · Fixed by #73
Closed

Exclude option must be interpreted relative to cabal file #71

konn opened this issue May 14, 2024 · 6 comments · Fixed by #73
Labels
bug Something isn't working

Comments

@konn
Copy link
Contributor

konn commented May 14, 2024

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 tfausak added the bug Something isn't working label May 14, 2024
@tfausak
Copy link
Owner

tfausak commented May 14, 2024

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:

excludedFiles = Set.fromList $ fmap normalize strs

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:

Hspec.it "reads from an input file" $ do

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

@fendor
Copy link

fendor commented May 14, 2024

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
Copy link
Owner

tfausak commented May 14, 2024

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

@fendor
Copy link

fendor commented May 14, 2024

@konn
Copy link
Contributor Author

konn commented May 17, 2024

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
Copy link
Contributor Author

konn commented May 17, 2024

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

tfausak pushed a commit that referenced this issue May 17, 2024
* test: cabal-gild must treat excluded paths relative to cabal file

* fix: treat excluded paths relative to cabal file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants