Re: haskell-src-exts Question

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
>
>