Hi Adding brackets that MUST have been there, by default, sounds like a great idea. The alternative is getting it wrong, so I think that's very safe.
Adding brackets that MIGHT have been there is a lot less clear cut. One important consideration is that the fixities you parse/pretty-print with might be wrong, so it has to be sensitive to that. You have the options: * Always do it (but then you get way too many brackets, and in the case where you mis-guess the fixities, you break the code) * Do it based on a table of fixities (might work if the parser fixities match the pretty-printer fixities, but could go wrong) * Annotate operators with fixities (this seems really wrong, and suffers from incorrect guessed fixities very badly) * Never do it My preference would be: -- put in enough brackets based on a fixities ensureEnoughBrackets :: [Fixities] -> a -> a prettyPrint = show . ensureEnoughBrackets [] Always do the safe brackets, if people want to do a table-of-fixities approach they can easily do so. Also by putting this code in the pretty printer it's harder to reuse if you want to write a custom pretty print or similar - ensureEnoughBrackets may be independently useful. Thanks Neil > 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? Yes - that seems perfectly sensible. > 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 > > _______________________________________________ Haskell-Cafe mailing list Haskell-Cafe@haskell.org http://www.haskell.org/mailman/listinfo/haskell-cafe