On Sun, 2011-08-14 at 14:22 +0100, Johan Tibell wrote:
> On Sun, Aug 14, 2011 at 1:52 PM, Paterson, Ross <R.Paterson@city.ac.uk> wrote:
> > Johan Tibell writes:
> >> This is a call for consensus. Do we agree to add
> >>
> >> infixr 6 <>
> >>
> >> (<>) :: Monoid m => m -> m -> m
> >> (<>) = mappend
> >
> > As I recall, the only remaining issue is that this operator is
> declared as infixl 6 in the pretty package. Someone needs to
> investigate the impact of changing its fixity there.
>
> Already done here: http://hackage.haskell.org/trac/ghc/ticket/3339#comment:22
>
> Seems to be a slight improvement actually.

So I was preparing to commit this change in base and validating ghc when
I discovered a more subtle issue in the pretty package:

Consider

a <> empty <+> b

The fixity of <> and <+> is critical:

(a <> empty) <+> b
= { empty is unit of <> }
(a ) <+> b

a <> (empty <+> b)
= { empty is unit of <+> }
a <> ( b)

Currently Text.Pretty declares infixl 5 <>, <+>. If we change them to be
infixr then we get the latter meaning of a <> empty <+> b. Existing code
relies on the former meaning and produces different output with the
latter (e.g. ghc producing error messages like "instancefor" when it
should have been "instance for").

Suggestions?

Don't use Monoid.(<>) in pretty for now, until someone has time to think about how pretty fits into a world with Monoid exporting <>.

-- Johan