darcs

Patch 1559 Add --[no-]enum-patches to show repo

Title Add --[no-]enum-patches to show repo
Superseder Nosy List gpiero
Related Issues
Status accepted Assigned To
Milestone

Created on 2017-04-20.15:41:04 by gpiero, last changed 2017-08-09.17:21:34 by gh.

Files
File name Status Uploaded Type Edit Remove
add-____no__enum_patches_-to-show-repo.dpatch gpiero, 2017-04-20.15:41:03 application/x-darcs-patch
patch-preview.txt gpiero, 2017-04-20.15:41:03 text/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg19473 (view) Author: gpiero Date: 2017-04-20.15:41:03
After the last refactoring, show repo does not accept '--no-files' anymore as
a way to perform the command in O(1). This has an impact on some hook scripts
of mine that invoke darcs show repo just for getting the repodir.
On the 4/5-years-old low-end (but with ssd) laptop I'm writing this on, the
show repo on darcs screened is now ~600ms instead of ~30ms:

$ time darcs show repo >/dev/null                                                                           
real    0m0.578s
user    0m0.528s
sys     0m0.040s
$ time darcs show repo --no-enum-patches >/dev/null                                                         
real    0m0.029s
user    0m0.012s
sys     0m0.012s

While it doesn't seem a great difference (and it isn't in general) this means
my hook script went from almost-no-delay to noticeable-delay.

Apart from the questionable name of the option, I'm asking for opinion about
the content of the patch itself: I've tried to conform to the recent (in
progress) refactoring (namely the use of YesNo types) but I'm not sure got it
correctly as I've ended up with a patch that seems unnecessarily convoluted.

1 patch for repository valentina:var/repos/darcs/screened:

patch 66da8f96aaa122f28546d239e3ad39b4867b9eb2
Author: Gian Piero Carrubba <gpiero@rm-rf.it>
Date:   Wed Apr 19 18:49:32 CEST 2017
  * Add '--[no-]enum-patches' to show repo
  for displaying patches count and weak hash (enabled by default).
  This is intended for executing show repo in O(1) when that data
  is not needed.
Attachments
msg19474 (view) Author: bf Date: 2017-04-20.16:05:07
Thanks, the patch is perfectly fine. The name --enum-patches may not be
perfect but it's not so bad either. Thanks for taking the time to update
the help text and for giving a good description for the option.

I'll accept as soon as its dependencies are accepted.
msg19526 (view) Author: bf Date: 2017-08-09.13:25:14
The patches this one depends on (and which aren't accepted yet) are:

patch 21efdfd6bbc5fed9f2b71b1f92619d5fc94ad404
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Apr  1 13:12:21 CEST 2017
  * make option definitions local to commands where possible
  
  The rationale is that this allows to omit type signatures. These
signatures
  are useless in practice and a hindrance whenever options are
refactored. The
  change is completely mechanical except where we share option definitions
  between related commands.
Shall I pull this patch? (2/3)  [ynW...], or ? for more options: w
patch 66da8f96aaa122f28546d239e3ad39b4867b9eb2
Author: Gian Piero Carrubba <gpiero@rm-rf.it>
Date:   Wed Apr 19 18:49:32 CEST 2017
  * Add '--[no-]enum-patches' to show repo
  for displaying patches count and weak hash (enabled by default).
  This is intended for executing show repo in O(1) when that data
  is not needed.
msg19527 (view) Author: bf Date: 2017-08-09.13:27:19
Sorry, ignore the last message.

Here are the not-yet-accepted patches that this one depends on:

patch 704f473f35d8c72b8037b641375e9a25ca100010
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Wed Mar 29 23:04:40 CEST 2017
  * get rid of almost all flag list hacking
  
  The only command where this was highly non-trivial was whatsnew. For the
  others, a few simple refactorings in the option definitions made the
changes
  quite mechanical. For instance, I have changed options where the user code
  (the command implementations, mostly) had to supply a default, with
separate
  options (e.g. conficts -> conflictsYes, conflictsNo). This makes it
clearer
  which commands have which defaults and makes it less probable to
  accidentally use one default for the options definition and another
one for
  parsing the option.
  
  The remaining few cases are those where we need to do processing of
the raw
  flag list: the match option hack for init (--to-match is converted to
  --match), and the WorkRepo and NewRepo flags. My plan for removing
these is:
  (1) change definition of the matching options darcs init takes, and
  (2) but IO into DarcsOptDescr, so we can do the needed processing on
  options, rather than the raw flag list

patch 21efdfd6bbc5fed9f2b71b1f92619d5fc94ad404
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Apr  1 13:12:21 CEST 2017
  * make option definitions local to commands where possible
  
  The rationale is that this allows to omit type signatures. These
signatures
  are useless in practice and a hindrance whenever options are
refactored. The
  change is completely mechanical except where we share option definitions
  between related commands.
History
Date User Action Args
2017-04-20 15:41:04gpierocreate
2017-04-20 16:05:07bfsetstatus: needs-screening -> review-in-progress
messages: + msg19474
title: Add '-- -> Add --[no-]enum-patches to show repo
2017-08-09 13:25:15bfsetmessages: + msg19526
2017-08-09 13:27:19bfsetmessages: + msg19527
2017-08-09 17:21:34ghsetstatus: review-in-progress -> accepted