darcs

Patch 1179 Darcs.Repository.Flags: added Eq instanc... (and 4 more)

Title Darcs.Repository.Flags: added Eq instanc... (and 4 more)
Superseder Nosy List bfrk, ganesh
Related Issues
Status accepted Assigned To ganesh
Milestone

Created on 2014-07-04.10:47:12 by bfrk, last changed 2014-11-04.23:03:10 by ganesh.

Files
File name Status Uploaded Type Edit Remove
darcs_repository_flags_-added-eq-instance-for-useindex.dpatch bfrk, 2014-07-04.10:47:12 application/x-darcs-patch
integrate-new-options-subsystem.dpatch ganesh, 2014-11-04.22:48:55 application/x-darcs-patch
intermediates.tar.gz ganesh, 2014-11-04.22:55:53 application/x-gzip
patch-preview.txt bfrk, 2014-07-04.10:47:12 text/x-darcs-patch
patch-preview.txt ganesh, 2014-11-04.22:48:55 text/x-darcs-patch
unnamed bfrk, 2014-07-04.10:47:12
unnamed ganesh, 2014-11-04.22:48:55
See mailing list archives for discussion on individual patches.
Messages
msg17591 (view) Author: bfrk Date: 2014-07-04.10:47:12
Hi Everyone

Please dis-regard the previous send, I forgot to pass --edit-description...

These patches implement my new options subsystem. Apologies for the huge
integration patch. I am aware that this makes it difficult to review the
changes but I could not figure out how to split it into smaller pieces while
still maintaining a working or even buildable darcs. I rebased my changes
several times: first to keep up with other changes in screened, and second
because the design of the new system went through a number of re-factorings.
I think one large patch is still better than the "real" series of changes I
made over time. If someone has a good idea how to split the patch in a
sensible way, please come forward.

On the good side, all the tests succeed, which means I did not make too many
grave errors ;-) Also, the second patch (which adds the new options
subsystem) does not affect the rest of the darcs, so the integration patch
can be amended w/o touching it. More importantly, there is now the
possibility to get rid of all the low-level scanning and manipulating of the
list of DarcsFlag in a step-by-step fashion: the basic mechanisms are in
place and just have to be used throughout, something I did not complete,
partly because I ran out of steam and partly because I wanted to give you the
opportunity to intervene, before I make even more sweeping changes.

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

Fri Jun 13 11:31:30 CEST 2014  Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
  * Darcs.Repository.Flags: added Eq instance for UseIndex

Sun Jun 29 13:23:23 CEST 2014  Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
  * Darcs.Repository.Flags: added Show instances for all types

Mon Jun 30 01:54:43 CEST 2014  Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
  * replaced duplicate DiffAlgorithm in Darcs.Repository.Flags with re-export

Mon Jun 30 01:47:27 CEST 2014  Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
  * added the re-designed options system, not yet used anywhere

Mon Jun 30 02:14:09 CEST 2014  Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
  * integrate new options subsystem
  
  The conversion is complete in the sense that the old Darcs.UI.Arguments is
  now obsolete and can be removed. Command options are specified using the new
  "typed" DarcsOption type from Darcs.UI.Options. All available tests succeed.
  
  However, it is incomplete in the sense that there are still many places
  where the list of DarcsFlag is scanned or manipulated directly. Removing
  these direct accesses will be an on-going effort; when it is complete we can
  replace [DarcsFlag] with an abstract data type.
  
  This patch contains an almost complete re-implementation of
  Darcs.UI.ArgumentDefaults under the new name Darcs.UI.Defaults. This adds a
  new dependency on regex-applicative, see the comments in the code.
  
  I have also made the tentative change of using a command specific
  configuration record for two of the commands, namely "record" and "amend".
  It remains to be seen whether this is the right approach and the other
  commands should follow it.
Attachments
msg17732 (view) Author: gh Date: 2014-11-03.18:52:57
Hi Ben,

I accepted the first three patches (Ganesh screened them) which are
cleanups:

* Darcs.Repository.Flags: added Eq instance for UseIndex
* Darcs.Repository.Flags: added Show instances for all types
* replaced duplicate DiffAlgorithm in Darcs.Repository.Flags with re-export

If you want to amend and complete the other two patches, go ahead. They
will probably not go in 2.10 which I'm branching very soon.
msg17739 (view) Author: ganesh Date: 2014-11-03.20:51:30
We really should have put this all in screened immediately, it's at 
least partially my fault that we didn't. I'm currently resolving 
conflicts and reviewing at the same time. We can discuss whether it 
makes sense for 2.10 or not if/when I'm done.
msg17744 (view) Author: ganesh Date: 2014-11-04.22:48:55
I've rebased Ben's patches against HEAD. I merged into one to make
the rebasing easier and also because it seemed a bit more logical that
way, otherwise there's an intermediate state where both old and new
systems have to compile.

It's nice that everything about an option lives in one place now,
and how to add a new option is somewhat clearer. The naming conventions
are also less confusing.

The negatives are that it's a bit more complicated than the old system
and the type signatures for the sets of options in each command are
pretty verbose. We could just leave them out but I don't know how to
disable to compiler warning just for those functions.

The patch also breaks the 'convert.sh' test. I think it's because
~/.darcs/defaults is overriding command-line options now. Ben's
original patches also have that problem but it didn't show up at
the time because the convert.sh test wasn't running.

I'm going to push this to screened and reviewed now and will fix
the convert.sh test in the next few days.

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

Tue Nov  4 22:37:57 GMT 2014  Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
  * integrate new options subsystem
  
  The conversion is complete in the sense that the old Darcs.UI.Arguments is
  now obsolete and can be removed. Command options are specified using the new
  "typed" DarcsOption type from Darcs.UI.Options. All available tests succeed.
  
  However, it is incomplete in the sense that there are still many places
  where the list of DarcsFlag is scanned or manipulated directly. Removing
  these direct accesses will be an on-going effort; when it is complete we can
  replace [DarcsFlag] with an abstract data type.
  
  This patch contains an almost complete re-implementation of
  Darcs.UI.ArgumentDefaults under the new name Darcs.UI.Defaults. This adds a
  new dependency on regex-applicative, see the comments in the code.
  
  I have also made the tentative change of using a command specific
  configuration record for two of the commands, namely "record" and "amend".
  It remains to be seen whether this is the right approach and the other
  commands should follow it.
  
  Rebased by Ganesh Sittampalam <ganesh@earth.li> to integrate with the latest
  darcs code. Originally in two patches that first introduced and then
  switched to the new code; I merged them into one to make the rebasing easier.
Attachments
msg17746 (view) Author: ganesh Date: 2014-11-04.22:55:53
For future reference in case I screwed up the rebase, attaching my 
intermediate working.
Attachments
History
Date User Action Args
2014-07-04 10:47:12bfrkcreate
2014-11-03 18:52:58ghsetstatus: needs-screening -> followup-in-progress
messages: + msg17732
2014-11-03 20:51:30ganeshsetmessages: + msg17739
2014-11-04 22:48:56ganeshsetfiles: + patch-preview.txt, integrate-new-options-subsystem.dpatch, unnamed
messages: + msg17744
2014-11-04 22:52:52ganeshsetstatus: followup-in-progress -> accepted-pending-tests
2014-11-04 22:55:53ganeshsetfiles: + intermediates.tar.gz
messages: + msg17746
2014-11-04 22:56:01ganeshsetassignedto: ganesh
nosy: + ganesh
2014-11-04 23:03:10ganeshsetstatus: accepted-pending-tests -> accepted