[darcs-devel] darcs patch: rewrite ColourPrinter with a policy data type (fixes R...

David Roundy droundy at abridgegame.org
Mon Apr 4 05:35:59 PDT 2005


On Sun, Apr 03, 2005 at 03:27:31PM -0400, ptp at lysator.liu.se wrote:
> Here is a fix for RT#302 - blue escapes in pipe
> 
> Ok, it does a little more... :-)
> 
> I am about to improve the control of escaping of control chars,
> and I thought I'd implement some of the extra tab/space/eol
> coloring/highlighting at the same time.  So I needed a bit
> more control in the ColourPrinter, and I took the chance to
> practice on the "policy thing" as well, before I trash^Wimprove
> the with_selected_changes interface.
> 
> My worry (if any) is I might have unknowingly broken some
> careful optimization.

Overall, it looks good, and although I see places it could be optimized
(see below) it doesn't look like you've broken any important optimizations.

> +getPolicy :: IO (Policy)
> +getPolicy =
> + do isTerminal <- hIsTerminalDevice stdout
> +    nColors <- getTermNColors
> +    return Policy { poColor  = isTerminal && (nColors > 4),
> +                    poEscape = True
> +                  }

Here we could use nested ifs to avoid the call to getTermNColors if we
aren't looking at a terminal.  It's probably not important, but there are
times when we print a lot of patches.

> +-- printers
> +
> +fancyPrinters :: IO (Printers)
> +fancyPrinters =
> + do po <- getPolicy
> +    return Printers { colorP     = colorPrinter po,
> +                      invisibleP = invisiblePrinter,
> +                      defP       = escapePrinter po
> +                    }

Here we could use unsafePerformIO to avoid calling getPolicy many times,
which would also move fancyPrinters out of the IO monad.  It's a tad ugly,
but would have the (probably totally unimportant) advantage of saving one
call to isTerminal and one to getTermNColors per patch printed.

Note that we could also put an unsafePerformIO in getPolicy, which would
move both getPolicy and fancyPrinters out of the IO monad (at the cost of
some ugliness).  Note that if you used this trick, you'd probably also want
to (or have to?) add a NOINLINE pragma.
-- 
David Roundy
http://www.darcs.net




More information about the darcs-devel mailing list