Hi Duncan, it took me a long time to return to the issue, but I'm very busy at work (which by fortune is an industrial Haskell project). Since only few points are in question, I wipe away all the other stuff to make it more clearly arranged.
5. This is connected to the CondTree structure. When I write a Cabl file like: ... library extensions: ForeignFunctionInterface if flag(use_xft) build-depends: X11-xft >= 0.2, utf8-string extensions: ForeignFunctionInterface ... the extension ForeignFunctionInterface in the conditional branch is superflous, as it is specified in any case, so the printer can simplify it to: ... library extensions: ForeignFunctionInterface if flag(use_xft) build-depends: X11-xft >= 0.2, utf8-string ... However the parse result for the two cabal files will differ. Now I gave up here, and added a variable simplifiedPrinting which disables the printer smartness for now. However, I hope someone can make the parser smarter in this respect.
I think it is better to stick to the syntax when pretty printing. If we want to do semantic simplifications that should be done separately.
So should simplifiedPrinting be True or False? Currently it says:
-- | Recompile with false for regression testing simplifiedPrinting :: Bool simplifiedPrinting = False
I think we should then keep False as default.
6. If free text starts with a line with a dot, the dot must be in a new line.
Right. In future we're going to allow free text without the silly dot stuff. I wonder how we'll handle things then.
7. Version tags have to be printed, which was not the case before (why?).
Actually that was on purpose. We must not print tags as they make no sense at all. Tags are a crazy idea that doesn't work at all. Some old packages use tags and there's not a lot we can do about that, but we have to ignore them or we'll go insane.
So I cannot accept this bit of the patch. If you have another suggestion I'm interested to hear. Perhaps we should ignore them when parsing too. Would that work?
Ignoring them when parsing would make the tests succeed in all cases. (Anyhow, all tests succeed now with newest Hackage cabal files and latest Cabal version, were my tag printing patch has disappeared?!)
9. Compiler options have to be put in a canonical order after parsing, which is easy as CompilerFlavors do derive Ord.
Why is this? Can we not preserve the order of the original cabal file?
That's true. I can take out reordering without problems.(attached patch)
Would you like to send in the test code as a patch, to add to the Cabal testsuite? That way we can make sure the round-trip parser/pretty-printer property remains true in future. Ideally we would make the test program use the hackage tarball index and check all of them.
I can do this, but were do I find the Cabal self testsuite? I attach my test file in case you want to help with this. I have to download the file from Hackage manually.
PS. I have added a remark about the new test suite syntax below.
Appendix 3: I find the test suite syntax problematic, for an exe I have to remeber the combination:
test-suite foo type: exitcode-stdio-1.0 main-is: main.hs
, and for a module:
test-suite bar type: library-1.0 test-module: Bar
So its the combination of exitcode-stdio- and main-is or library- and test-module which does the trick. Wouldn't it be easier to write something like:
test-suite foo test-version: 1.0 main-is: main.hs , and for a module: test-suite bar test-version: 1.0 test-module: Bar
You mean you want to make the test type implicit, depending on which fields are specified? I think it's important to specify the test type because there are not necessarily just two. There are two now but the point of the design is to allow for test protocols that we have not yet thought of, as we get more experience with the system. Those new protocols may well reuse the same fields, so the fields used may not uniquely determine the test type.
Note that the current Cabal code will remind you of the right fields, depending on the test type that you specify. It also tells you which test types there are.
Ok, I just can say that the combination of test type and version in combination with the other fields was a little difficult for me to get. So I add the patch to get rid of reordering compiler options. The priniting of version tags has gone anyway, after I updated to newest version. Jürgen