On Fri, 2009-11-13 at 23:54 +0100, Niklas Broberg wrote:
Surely you do want this. It's the biggest problem with the original haskell-src package, that it cannot print out any useful Haskell code obtained from the parser, because it forgets all the brackets.
I should point out that haskell-src-exts already fixes this for code obtained from the parser, by making the parser and AST remember the brackets. Or as you put it:
It probably wants to be a combination of the parser, AST and pretty printer.
Yes indeed.
Ok, I misunderstood.
But the problem at hand here is auto-generated AST code, where we cannot rely on the parser to do the right thing. There's help in the AST such that it's possible to explicitly insert brackets where needed, but I agree with Dominic that it shouldn't really be necessary in his case.
Neil's point is well taken though - to do it correctly (or rather, minimally) for infix application, the pretty printer would need to be aware of the respective fixities involved.
To do it minimally yes, but correctly? In the AST you've got InfixApp Exp QOp Exp so we know the tree structure, we just can't insert minimal brackets without knowing the fixity.
However, that doesn't mean we can't do better than what it is now, but be conservative about it. Only insert brackets where it's clear that brackets must be inserted, which would be the case for Dominic's example. If the argument to an application is non-atomic, it needs brackets, there's nothing ambiguous about that. Nothing can be said so categorically for infix applications, so there we should assume that the fixities are already done in the correct way, or that brackets are inserted manually where needed.
Does that sound reasonable?
So to be clear, currently the printing behaviour is that no brackets are inserted for infix expressions which means the results are "wrong by default" (for ASTs constructed manually, not via the parser) but on the other hand this lets you insert the minimal (or pleasing) number of brackets. The suggestion is to move to a "safe/correct by default" where brackets are inserted to preserve the tree structure of infix expressions. The problem then becomes, what if we want to have the minimal (or pleasing not-quite-minimal) number of brackets. Right? If I've understood right, then yes I think making the pretty printing right by default is a good idea, and then for the users/applications where optimising for fewer brackets is important, it should be a little extra work to supply the necessary information. Perhaps like the ParseMode has fixities :: [Fixity], give the PPHsMode the same (partial) fixities environment. For operators not in the environment we fall back to using brackets all the time, but for known operators we can the use minimal bracketing. Another option I suppose would be to annotate the QOp used in InfixApp with a Maybe fixity. The parser would annotate these when it knows them from its fixities env in the ParseMode. For ASTs constructed manually the user would add them if they know or care. If not, they just get slightly more brackets than might strictly be necessary if the fixity were known. Duncan