Mostly trivial cleanups and refactors in various command implementations

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

patch ebc495dbca97ec9d733c0503812cd9f5f6f43fc0
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Mar 24 07:06:33 CET 2021
  * remove out-commented code in D.UI.C.Repair

patch c7b43eb53ffa0ee1552e3f56d8e8633b7155904e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Mar  6 09:31:56 CET 2021
  * fix TODO item in remove command

patch ab5d1a112910ff903001edb1fbdd5e91b4e1c232
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Mar  6 16:57:55 CET 2021
  * push command: simplify some type signatures

patch 02c1b80ddae63ea9a844c779fc597e6809b3346a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Mar  6 09:13:07 CET 2021
  * remove command: fix code layout of makeRemovePatch

patch 73cb1f042cec8b3918769124c2a387a4bd3823a9
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Jun 19 09:27:40 CEST 2021
  * rename PatchTree to PatchSeq

  This is about the PatchTree defined by the implementation of the test
  command. The renaming was done because semantically it is a sequence with
  guaranteed O(1) concatenation.

patch cfcff3bc226c2fa01dccc70b7a46a6ac284749ac
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Jun 22 14:43:51 CEST 2021
  * re-format Darcs.UI.Commands.Send.sendBundle

patch 9be18517a82f60ab3ba65b617a1aeda6a983ba6d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Jul 22 20:48:36 CEST 2021
  * inline some trivial functions in D.UI.Cs.Unrecord

patch 80dbe2e2dc8b845bb8ffb58673dc66fba7ea4ccb
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Sep 18 11:07:05 CEST 2021
  * replace: allow only printable ASCII characters in the token spec

  This enforces a limitation that was previously not checked and could lead to
  corrupt patches.

patch 902c2a15fce972ccc314772d9e8377b728db5b5f
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Sep 13 11:40:29 CEST 2021
  * push: replace parseFlags with ?-operator

patch fb46be6c5f7021e718f5944221539fb0aded5c08
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Nov 16 11:15:43 CET 2021
  * whatsnew -u: print output with pager if non-interactive

Attachments

• patch-preview.txt
• remove-out_commented-code-in-d_ui_c_repair.dpatch

>  * remove out-commented code in D.UI.C.Repair

OK

>   * fix TODO item in remove command
> -   -- TODO whether command fails depends on verbosity BAD BAD BAD

:-)

>   * push command: simplify some type signatures

OK. For what it's worth, working with type variables explicitly is
becoming more common in Haskell as support gets better. Having the
explicit foralls can be useful for callers to fix the order of type 
variable arguments when using `TypeApplications`. But it's no big
deal to put them back in future.

>   * remove command: fix code layout of makeRemovePatch
>   * rename PatchTree to PatchSeq
>   * re-format Darcs.UI.Commands.Send.sendBundle
>   * inline some trivial functions in D.UI.Cs.Unrecord
>   * replace: allow only printable ASCII characters in the token spec
>   * push: replace parseFlags with ?-operator
>   * whatsnew -u: print output with pager if non-interactive

OK

Re: explicitly mentioning type variables using 'forall'. Your objection has 
been noted and I'll refrain from removing redundant foralls unless I have to 
re-write the type signature anyway. I think you said something like this 
before in a patch review but I believe this patch still predates that 
previous remark.

Your argument is valid, no question about that. But it is also true that 
explicit 'forall' adds noise and therefore makes it harder to read/understand 
code at a glance. It also adds extra overhead during refactors: Turn a pair 
(p :> q) into two arguments, bang, type error because you forgot to 
explicitly list the intermediate wXYZ. (It would be okay if this were only a 
warning, but it's an error.) I am not the first to note that 
ScopedTypeVariables are not a well-designed feature.

Yeah, I agree they look ugly. I don't feel particularly strongly about
removing them if the code still compiles, just noting we might find
ourselves re-adding them in future.