Line trailing comments not stripped

[phadej]

issue with accelerate-cuda 0.14.0.0 file: https://gist.github.com/phadej/641d82190cdd0400fc28

Original:

Extra-tmp-files:        config.status
                        config.log
                        autom4te.cache
                        accelerate-cuda.buildinfo               -- generated by configure

Formatted:

extra-tmp-files:
    config.status
    config.log
    autom4te.cache
    accelerate-cuda.buildinfo
    --
    generated
    by
    configure

---

I'm not sure if the issue is in the parser or prettyprinter

---

related to: #2460 #2607

[phadej]

Test case in phadej@836d414

[BardurArantsson]

This issue desperately needs a description of what the actual problem is, i.e. what did you do and what did you expect to happen? What did happen?

[23Skidoo]

This issue desperately needs a description of what the actual problem is, i.e. what did you do and what did you expect to happen?

He took accelerate-cuda-0.14.0.0.cabal and ran it through cabal format. See #2607 (comment).

[BardurArantsson]

Yes, I can see that, but this is still not a useful issue description if a reader of the issue has to trawl through comments in a different issue/PR to figure out what's going on. (Esp. since the PR is still open.)

EDIT: grammer

[lspitzner]

@BardurArantsson without any deeper knowledge about the subject, here is my guess what is meant:

-- generated by configure is a comment. it is not kept intact by a parse/pretty-print round-trip.

the expected behaviour is that
a) the output contains the complete comment -- generated by configure
b) most importantly: there are not three new extra-temp-files, namely "generated" "by" and "configure"

[BardurArantsson]

Ok, that's a more useful description. It shouldn't be necessary to guess, however since it would take all of 5 minutes for the reporter to write that :).

( @phadej : I promise, I'm not trying to single you out specifically (... at length). I'm just making the point that for issue descriptions to be useful they must be understandable by people who aren't involved in the original issues, etc. IOW understandable on their own. You just happened to provide an example. :))

[phadej]

I investigated this further. The problem is in the parser, it doesn't strip -- trailing line comments.
For example cabal file with

license: BSD3 -- or should we have GPL?

will fail to parse, which is ok.

But list fields (like extra-source-files) will be parsed so comment as treated as real content:

extra-src-files: README.md -- foobar

into

extraSrcFiles = ["README.md","--","foobar"],

---

There is trimLines in Cabal.Distribution.ParseUtils which strips whole line comments, but it cannot be extended as context is important. Eg. we would like to have

cc-options: --my-special-compiler-option

---

Hacky soluton A:

I could make abit hacky solution to strip WS+ DASH DASH WS+ (not DOUBLEQUOTE)* from the end of the lines in trimLines.

---

Real solution B: would be to take @dcoutts parsec parser into use, but it is blocked by ghc-the-library dependency on Cabal. I could help with decoupling, if there are some pointers.

[phadej]

Update. This is by design. The frame syntax of .cabal like files doesn't accept end-of-line comments.

I'm not sure how to proceed. Some warning would be useful. Maybe warn about, --files, and suggest using "--files" in some fields (files, and modules, build-depends?)

ping @hvr