patch: pretty printing for Cabal

[Jürgen Nicklisch-Franken]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pp3.patch
Type: text/x-patch
Size: 115955 bytes
Desc: not available
Url : http://www.haskell.org/pipermail/cabal-devel/attachments/20101115/adb7912f/pp3-0001.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Main.hs
Type: text/x-haskell
Size: 2726 bytes
Desc: not available
Url : http://www.haskell.org/pipermail/cabal-devel/attachments/20101115/adb7912f/Main-0001.bin