Add verbosity patterns#11778
Conversation
philderbeast
force-pushed
the
refactor/pattern-ConfigVerbosity
branch
from
047decd to
18fa759
Compare
| -- | Same as 'CommonSetupVerbosity', but using the default verbosity handles and | ||
| -- 'fromFlagOrDefault' to get the verbosity from the flags. | ||
| pattern DefaultCommonSetupVerbosity :: Verbosity -> CommonSetupFlags | ||
| pattern DefaultCommonSetupVerbosity v <- (mkVerbosity defaultVerbosityHandles . fromFlagOrDefault normal . setupVerbosity -> v) |
There was a problem hiding this comment.
I am uncomfortable with anything that bakes in defaultVerbosityHandles. I know it is tedious to see it a lot in the code but any code that makes use of it is wrong if it can be called as a library function: it bakes in stdout/stderr as the output location for logging. This is only OK if we know we are going to redirect them, e.g. via handle redirection in a separate process.
As far as I know, most of the uses are in old v1-code or in Setup.hs files, and I do not want the usage to proliferate at all. If you insist on this pattern synonym then it should be named appropriately to avoid accidental usage, which can easily lead to frustrating bugs in which some log output is missing because it was not properly redirected to the logging handle.
There was a problem hiding this comment.
It is a shame that I've squashed my commit history. I'd originally only had DefaultCommonSetupVerbosity as a private pattern in cabal-install/src/Distribution/Client/Main.hs but then saw that it could be used in other modules too:
cabal-install/src/Distribution/Client/CmdLegacy.hscabal-install/src/Distribution/Client/CmdHaddockProject.hs
To make it accessible to these other modules, I moved it to the same place as CommonSetupVerbosity, in Cabal, because these patterns seem related but I'd be happy to move DefaultCommonSetupVerbosity back to cabal-install, either privately in Main as before or somewhere accessible to the other modules in cabal-install that are using it now.
sheaf
left a comment
sheaf
left a comment
There was a problem hiding this comment.
I agree that the current handling of Verbosity vs VerbosityFlags is a bit frustratingly noisy, so I appreciate the effort to tidy it up a bit. I'm not entirely convinced by the current approach, but I don't really have a good suggestion to make at this point I'm afraid.
- Add mkConfigVerbosity - Add CommonSetupVerbosity pattern - Use CommonSetupVerbosity - Add Default*Verbosity patterns - Move DefaultCommonSetupVerbosity to Main - Move *Verbosity patterns - Use CommonSetupVerbosity in Simple module - Use CommonSetupVerbosity in Register module - Use CommonSetupVerbosity in Install module - Use CommonSetupVerbosity in SrcDist module - Use CommonSetupVerbosity in Test module - Use CommonSetupVerbosity in Haddock module - Use CommonSetupVerbosity in ExeV10 module - Use CommonSetupVerbosity in LibV09 module - Use Default*SetupVerbosity in CmdLegacy - Add haddocks to *Verbosity patterns - Use Default*Verbosity pattern in CmdHaddockProject
philderbeast
force-pushed
the
refactor/pattern-ConfigVerbosity
branch
from
18fa759 to
d34c3b9
Compare
2 participants
Fixes #11777.
CommonSetupVerbosityandDefaultCommonSetupVerbositypattern synonyms exported fromDistribution.Simple.Setup.ConfigVerbositypattern synonym private toDistribution.Simple.Configure.FlagVerbositypattern synonym private toDistribution.Make.This replaces longer repetitive code with shorter pattern matches. The more intricate code, moved inside the pattern synonyms, is not repeated.