Slightly random list of cleanups and refactorings in the UI subsystem. Most
of these are in preparation for the what I am really after, which is to get
rid of all explicit flag list hacking in darcs commands. This is something I
am *almost* done with (all tests pass) but the changes needs to be cleaned
up a bit before I send them.

Before I push these changes to screened, I want a bit of feedback wrt my
introduction of the (?) operator. Are there any strong objections to that?

13 patches for repository http://darcs.net/screened:

patch ec1ae7c9e9da46c94aa7173905af4bca7990b60e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 13:49:40 CEST 2017
  * rename (amQuiet,amVerbose) to (quiet,verbose), move to D.UI.Flags, and export

patch d6e6e52be2bb83c2bd860cb4b4c7d27e07551e1f
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 13:58:56 CEST 2017
  * renamed hasXmlOutput to xmlOutput, hasXmlOutput returns Bool

patch dac1adff5ff26089c73744fd79203abc103e2852
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 14:01:04 CEST 2017
  * cleaned up obsolete comment in Darcs.UI.Options.All

patch bc944b4beff4eac8da9702808ef3cd626b0c77e0
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 14:17:08 CEST 2017
  * cleanup of Darcs.UI.Options "master" module
  
  - use Functor instance for OptDescr since we require base >= 4.8 now
  - cleaned up imports, exports, and comments

patch affb24ef5fc76402c1d0a3c14ef35ccf673606d1
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 14:40:02 CEST 2017
  * cleaned up pull command
  - removed explicit flag list manipulation
  - use verbose, xmlOutput, putVerbose, etc
  - avoid excessively long type signatures for option definitions

patch 2ce523cc17e8d4cd2761f78d6d2915ac6f914e9c
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 14:52:19 CEST 2017
  * remove flag list manipulation from show {authors, repo}

patch 1e72adc96dbb1afc06c92270ed1720644228b240
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 15:37:16 CEST 2017
  * added infix operator (?) as synonym for parseFlags

patch 1624f79983ad88c6b22a34c511355a04df094218
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 15:39:54 CEST 2017
  * renamed option xmloutput to xmlOutput

patch 6278edf0ca35dd3e19112221c40007e0168c2522
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 20:52:19 CEST 2017
  * fix handling of parse errors for --max-count option

patch ba7c77db71deb99a9d1da9e70abcd5ba31c04b02
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 28 21:35:50 CEST 2017
  * refactor: replace all trivial functions in D.UI.Flags with re-exports
  
  This is a big patch but the changes are mostly trivial. Darcs.UI.Flags
  contained many functions that just extract the value of a primitive option.
  In many cases the functionality was exported under a name that slightly
  differed from the name of the option. All of these functions have been
  removed and replaced with re-exports of options from Dacs.UI.Options.All. In
  some cases the renaming was done in the other direction, i.e. the option was
  renamed to what the function in D...Flags was called.
  
  Re-exporting the options was done because the change is already large enough
  and also for convenience (moving explicitly imported names from one module
  to another for all the many commands is really tedious).
  
  The new (?) operator synonym for parseFlag was used heavily to adapt the
  command implementations.
  
  In a few places I cleaned up legacy code e.g. use putInfo and putVerbose.

patch f65420a5def30bacdbf5a149a7d6d4ead2fbe51e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Mar 30 00:48:37 CEST 2017
  * more option renaming: askdeps->askDeps, workingRepoDir->repoDir

patch c7f3c3cdc667ec9b30cb9c6c2e8357685e66858b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Mar 30 07:45:56 CEST 2017
  * add class YesNo and instances to Darcs.Options.All
  
  This is to make it less verbose to test the option specific data types, many
  of which are equivalent to Bool.

patch 3c07898bbffbd4d467f9bffbd239e3ef2819e4b0
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Mar 30 08:24:55 CEST 2017
  * removed unused option reorderPatches (all commands use option 'reorder')

Attachments

• patch-preview.txt
• rename-_amquiet_amverbose_-to-_quiet_verbose__-move-to-d_ui_flags_-and-export.dpatch
• unnamed

>   * rename (amQuiet,amVerbose) to (quiet,verbose), move to 
D.UI.Flags, and export
>  * renamed hasXmlOutput to xmlOutput, hasXmlOutput returns Bool
>  * cleaned up obsolete comment in Darcs.UI.Options.All
>  * cleanup of Darcs.UI.Options "master" module

Look fine.

> * cleaned up pull command

Looks fine.

>  - avoid excessively long type signatures for option definitions

One comment on moving these to where clauses to avoid having to write 
signatures - we could instead have top-level definitions without 
signatures, it's just a style choice to normally insist on those.

We'd need to suppress the warning somehow, either for the whole file, 
or maybe we can now use PartialTypeSignatures instead.

>  * remove flag list manipulation from show {authors, repo}
>  * added infix operator (?) as synonym for parseFlags
>  * renamed option xmloutput to xmlOutput

Look fine.

>  * fix handling of parse errors for --max-count option

Fine

>  * refactor: replace all trivial functions in D.UI.Flags with re-
exports

Looks fine.

The hunks look a bit funny because of the hasAuthor -> author renaming 
- darcs first used explicit hunks to replace author with hasAuthor in 
strings/comments so that the subsequent replace would be invertible - 
but the end result is correct.

>   * more option renaming: askdeps->askDeps, workingRepoDir->repoDir

Again some funny looking hunks in Record and Amend, because 'askDeps' 
was already there. In those cases I think the 'replace' was wrong (in 
that it didn't change any existing 'askdeps'), but now it's done it 
doesn't really matter.

>   * add class YesNo and instances to Darcs.Options.All

Sounds like a good idea, though I don't see any examples of it being 
used in this bundle.

>   * removed unused option reorderPatches (all commands use option 
'reorder')

Fine.

Am 17.04.2017 um 11:34 schrieb Ganesh Sittampalam:
>>  - avoid excessively long type signatures for option definitions
> 
> One comment on moving these to where clauses to avoid having to write 
> signatures - we could instead have top-level definitions without 
> signatures, it's just a style choice to normally insist on those.
> 
> We'd need to suppress the warning somehow, either for the whole file, 
> or maybe we can now use PartialTypeSignatures instead.

For most commands the 'local style' feels just right to me. There are a
few exceptions, mainly modules which define many subcommands or which
have non-trivial aliases (e.g. repair/check), where we want to share a
whole (sub-)set of options. These cases are currently few enough that i
don't think keeping the type signatures for them is going to be a problem.

Anyway if you know of a simple way to turn off warnings for specific
definitions, i'd like to hear about that.

Cheers
Ben

> Ganesh Sittampalam <ganesh@earth.li> added the comment:
> The hunks look a bit funny because of the hasAuthor -> author renaming 
> - darcs first used explicit hunks to replace author with hasAuthor in 
> strings/comments so that the subsequent replace would be invertible - 
> but the end result is correct.

It may be that I have botched these renames. As usual I had lots of
changes in my working tree and had to separate out unrelated changes
after I made them.

With 'darcs replace' this is more difficult than it has to be. The
problem is that replace changes are always offered last i.e. after
hunks. I would very much appreciate an option to present them before
hunks, so that I can select them (and hopefully automatically get all
--force backward renames as dependent changes) so i can more easily
record them separately.

> Sounds like a good idea, though I don't see any examples of it being 
> used in this bundle.

Is used extensively in later patches.

Cheers
Ben