Avi-D-coder / implicit-hie

Auto generate a stack or cabal multi component hie.yaml file
BSD 3-Clause "New" or "Revised" License
201 stars 17 forks source link

Package name not picked up #43

Open sergv opened 2 years ago

sergv commented 2 years ago

TLDR if I start package name with a newline then it's not picked up

So I indented my cabal file as follows

test1.cabal

cabal-version: 3.0

name:
  test
version:
  0.1

build-type:
  Simple

library
  exposed-modules:
    Foo
  hs-source-dirs:
    src
  build-depends:
    base >= 4.9

and got following from parsing

> parsePackage' <$> T.readFile "/tmp/test.cabal"
Right (Package "" [Comp Lib "" "src"])

However, if I change just one line then it parses (via Hie.Cabal.Parser) ok

test2.cabal

cabal-version: 3.0

name: test
version:
  0.1

build-type:
  Simple

library
  exposed-modules:
    Foo
  hs-source-dirs:
    src
  build-depends:
    base >= 4.9
> parsePackage' <$> T.readFile "/tmp/test2.cabal"
Right (Package "test" [Comp Lib "" "src"])

Since cabal accepts test1.cabal, I think it should be supported. What do you think?

Avi-D-coder commented 2 years ago

Thanks for the report. The problem is src/Hie/Cabal/Parser.hs:204. PRs are appreciated, otherwise I'll try to get to it this weekend.

sergv commented 2 years ago

I tried to come up with a quick fix that will just call following function instead of tabOrSpace:

tabOrSpaceOrNewline :: Parser Char
tabOrSpaceOrNewline = tabOrSpace <|> '\n' <$ endOfLine

But that lead to failure of "succesfully parses empty other-modules2" test. It seems that when one calls skipMany tabOrSpaceOrNewline (cf definition above) it will consume too much indentation on the next line. Let's call this point 1.

While doing the fix I tried to add new test and in the process noticed that for the following pretty innocent cabal file

cabal-version: 3.0

name:
  simple-exe
version:
  0.1
build-type:
  Simple

executable simple-exe
  default-language:
    Haskell2010
  hs-source-dirs:
    src1 src2
  main-is:
    exe/Main.hs

the parse result would be

> parsePackage' <$> T.readFile "/tmp/simple-exe.cabal"
Right (Package "simple-exe" [Comp Exe "simple-exe" "src1/exe/Main.hs",Comp Exe "simple-exe" "src2/exe/Main.hs"])

The issue is the inferred paths: "src1/exe/Main.hs" and "src2/exe/Main.hs". When package meant by such cabal file wil be built, at the package toplebel there would exsit at least 3 directories: exe, src1 and src2. Directories src1/exe and src2/exe would not exist. Let's call this point 2.

Besides, what if some fields we're looking for are stored in common stanzas and are included rather than written verbatim? Handling all the cases seems hard - point 3.

Given the points 1, 2 and 3 outlined above it seems to me that the best course of action is not to duplicate Cabal parsing logic but instead either use parser from the Cabal package (but this particular dependency cannot be added for reason's I don't yet understand) or avoid parsing at all like suggested in https://github.com/Avi-D-coder/implicit-hie/issues/42.

Avi-D-coder commented 2 years ago

#42 is definitely the way forward. This package was never meant to be a long term solution.