[darcs-devel] [patch2038] finally complete transition to option system

Ganesh Sittampalam bugs at darcs.net
Sun Jul 19 11:17:31 UTC 2020


Ganesh Sittampalam <ganesh at earth.li> added the comment:

On 23/06/2020 17:43, Ben Franksen wrote:

>   * eliminate DarcsFlag list hacking for NewRepo and WorkRepoDir

>   To make this more uniform, the init command now takes the O.reponame option
>   instead of O.repoDir, like all other commands that create a new repo (clone,
>   convert darcs2, convert import). This is even compatible since we still
>   support --repodir as an alias for --repo-name.
>   
>   Converting a normal argument to a DarcsFlag now uses the standard option API
>   i.e. ounparse, see the new function Darcs.UI.Flags.withNewRepo.

Looks ok - I haven't thought it through in detail but it sounds plausible.

>   * replace combined option workRepo with a plain function
>   
>   This option was artificial in that no command actually specified it. I
>   always found its presence in Darcs.UI.Options.All confusing.

OK

>   * rename option reponame to newRepo

OK

>   * remove DarcsFlag list hacking for match options

OK

>   * avoid use of matchAny in commands

OK (I'll assume the change is correct)

>   * fix: add missing cases in getMatchPattern

OK

>   * add comments about use of RemoteRepo DarcsFlag constructor

Thanks!

> +-- use of RemoteRepo data constructor is harmless here, if not ideal

This is a bit unclear without looking at the comment below about
accessing the flag list, and could get out of date if that gets changed.
But not a big deal.

>   * remove option type parameter from DarcsCommand
>   
>   This is basically a complete rollback of
>   
>   patch 6ef3e49796ef64ae23bde58bfc4ea6720e8fd14d
>   Author: Ganesh Sittampalam <ganesh at earth.li>
>   Date:   Fri Nov 14 07:18:20 CET 2014
>     * Make the options type used by a command into a type parameter.
>   
>   Made possible since we no longer do any low-level DarcsFlag hacking.

I barely remember the history of this, although the new state looks fine
to me. From what I recall/understand from this patch:

- We started transitioning from [DarcsFlag] to strongly typed option
records for each command, but didn't get very far and it's now on hold
or abandoned, but not rolled back. An example of a migrated command is
'amend'.

- I added this type parameter/explicit config translation step to make
it hard to forget to normalise [DarcsFlag] for commands that still use
it directly.

- Now we still have lots of commands that use [DarcsFlag] for options,
but because they treat it opaquely, the explicit normalise step isn't
needed.

Is that right?

I also find onormalise/defaultFlags in D.UI.Options.Core a bit confusing
now, although it's not directly touched by this patch. onormalise is now
only used by defaultFlags. Both of them have specifications that are
identical to their bodies. Perhaps there's some scope for simplification
here.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch2038>
__________________________________


More information about the darcs-devel mailing list