Not for screened yet, but this is one possible option for avoiding
possible problems with legacy [DarcsFlag] inspection until we have
cleaned it all out.

It breaks the utf8.sh due to issue2416.

1 patch for repository darcs-unstable@darcs.net:screened:

Fri Nov 14 06:18:20 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * Make the options type used by a command into a type parameter.
  
  The default for commands that haven't been migrated to the new
  options system is [DarcsFlag].
  
  The explicit parse step this change introduces provides a
  convenient point to normalise the [DarcsFlag] list for unmigrated
  commands, to avoid any problems caused by default values for
  options that are overridden by later choices appearing in the list.

Attachments

• make-the-options-type-used-by-a-command-into-a-type-parameter_.dpatch
• patch-preview.txt
• unnamed

I think this is a pretty clever idea. Yes, we have to carry the type
parameter through everywhere, or use an existential to hide it, but I
guess that is the price to pay and it is not too high. So a clear +1
from me.

At first I was a bit confused about WrappedCommand vs. CommandControl.
The former wraps a DarcsCommand, the latter also distinguishes between
normal commands, hidden commands, and so on. They are both GADTs now. As
an alternative, have you considered pushing the existential wrapping
down into the alternatives:

data CommandControl
  = CommandData WrappedCommand
  | HiddenCommand WrappedCommand
  | GroupName String CommandControl

This means we have to wrap them in the super command implementations
(but that could be done with a small helper function) and would avoid
having to unwrap and re-wrap them for some operations in Commands.hs.

BTW, can we find a better name for CommandControl and its data constructors?

I have attached a follow-up patch that implements what I suggested. Not
sure this is a definite improvement but judge for yourself.

Attachments

• make-the-options-type-used-by-a-command-into-a-type-parameter_2.dpatch

This also breaks some send tests, not sure why I didn't see those 
failures the first time round.

Those send failures are fixed by patch1230, and the utf8 failures by 
Ben's patch1231.

Resolving conflicts with mainline screened.

I think this bundle should go in once we have resolved the
outstanding confusion over the --to-patch argument, which
causes test failures in rebase-unsuspend-to-patch.sh and
workingdir.sh

With the WrappedCommand update I think it's a trade-off
of where the unwrapping and re-wrapping goes, but on
balance I agree it's probably easier to understand with
the update.

2 patches for repository darcs-unstable@darcs.net:screened:

Fri Nov 14 06:18:20 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * Make the options type used by a command into a type parameter.
  
  The default for commands that haven't been migrated to the new
  options system is [DarcsFlag].
  
  The explicit parse step this change introduces provides a
  convenient point to normalise the [DarcsFlag] list for unmigrated
  commands, to avoid any problems caused by default values for
  options that are overridden by later choices appearing in the list.
  

Tue Nov 18 07:11:31 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * resolve conflicts

Attachments

• make-the-options-type-used-by-a-command-into-a-type-parameter_.dpatch
• patch-preview.txt
• unnamed

This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/screened.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_screened.html#bundle-f26484eb2efe1696e438863ebc389d3ea7f054f2

I am accepting this and the follow-up patches.

When I push this to reviewed I get conflicts!

How can that be??? Whatsnew (in reviewed) says no changes. Pull (here)
says nothing to pull.

If I darcs get the reviewed branch and pull locally I also get
conflicts. I think this is a bug and a serious one at that. My best
guess ATM is that darcs somehow forgot some dependencies, either when
recording or when I applied the patch bundle. Hmm, could it be that this
has something to do with the context minimizing that has recently been
added?

Which patches were you trying to pull? My first one does have conflicts which are 
resolved by a followup, so if you try to pull one without the other then the conflicts 
are expected. You'd see the same in screened if you unpull the resolution patch and run 
'darcs mark-conflicts'.

That said, given issue2432, a bug in darcs is a definite worry.

My fault and not a Darcs bug. I did not expect a patch from screened to
have conflicts. Now that I think a bit about what you said, it is
obvious that this is what must happen if one pulls a conflicting patch
and not the conflict resolution patch. My only excuse is that in my
daily work I have a long time ago made it a rule never to push
conflicting patches to a shared repo; I always resolve conflicts by
rebasing.

Sorry for the alarm.

To answer your question, this is the patch:

patch 6ef3e49796ef64ae23bde58bfc4ea6720e8fd14d
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Fri Nov 14 07:18:20 CET 2014
  * Make the options type used by a command into a type parameter.

and indeed it has conflicts.

So, is there a way to find out which patch resolves this conflict so I
can push them together?

This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-f26484eb2efe1696e438863ebc389d3ea7f054f2