quchen / prettyprinter

A modern, extensible and well-documented prettyprinter.
BSD 2-Clause "Simplified" License
293 stars 34 forks source link

Add module hierarchy with shorter module names #174

Closed sjakobi closed 4 years ago

sjakobi commented 4 years ago

Addresses #110.


TODO:


Travis: https://travis-ci.org/github/quchen/prettyprinter/pull_requests

sjakobi commented 4 years ago

The best way to review this is probably simply to build the haddocks locally and browse them…

sjakobi commented 4 years ago

hlint makes CI fail. :(

prettyprinter-ansi-terminal/src/Prettyprinter/Render/Terminal/Internal.hs:178:17-42: Suggestion: Reduce duplication
Found:
  currentStyle <- unsafePeek
  let newStyle = style <> currentStyle
  push newStyle

Perhaps:
  Combine with prettyprinter-ansi-terminal/src/Prettyprinter/Render/Terminal/Internal.hs:240:17-42
sjakobi commented 4 years ago

I think I'll hold off on converting the rest until I'm more confident that we'll eventually remove the Data.Text.Prettyprint hierarchy.

sjakobi commented 4 years ago
-- | This module is part of the old @prettyprinter-ansi-terminal@ API which is
-- planned to be deprecated and eventually removed.
--
-- Use "Prettyprinter.Render.Terminal" instead.

I wonder whether "API" might be misunderstood to mean that the types and functions are changed too…

Maybe it would be better to say "module hierarchy" or "namespace"…

quchen commented 4 years ago

Looks good to me. Some of the deeper modules can’t be made super short, such as Prettyprinter.Render.Terminal.Internal, but then again those don’t regularly need to be referred to by the library’s users.

As for the »API«, I think »rather deep module naming hierarchy« is proably a bit verbose, but also very descriptive.

quchen commented 4 years ago

Timeframe-wise I think we should err on the safer side – there is no need to remove the deprecated modules for years, really. Developing the library has taught me a bit about #ifdef imports and how pleasant they are to use ;-)

sjakobi commented 4 years ago

Timeframe-wise I think we should err on the safer side – there is no need to remove the deprecated modules for years, really. Developing the library has taught me a bit about #ifdef imports and how pleasant they are to use ;-)

Indeed there's not much need to rush. The most significant downside to having both module hierarchies is that the package overview is somewhat cluttered. Nevertheless, I think we can keep up the deprecation for at least a year.

istathar commented 4 years ago

The most significant downside to having both module hierarchies is that the package overview is somewhat cluttered.

If you use

{-# OPTIONS_HADDOCK hide #-}

in a module header alongside the language pragmas it'll suppress that module from appearing in generated Haddock output. I use it to hide internal modules without making it impossible to access them. Works really well for deprecated modules, too.