Various fixes and cleanups (annotate, show xxx, options)

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

patch 5ae1b136e088bbf788982110a0d8818036d004f8
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Thu Mar 23 09:34:27 CET 2017
  * remove files and directories options from commands that don't use them

patch ac35dc465641a1ae261fb92b88c84dff2b082b4e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Thu Mar 23 11:09:33 CET 2017
  * annotate/options: allow --human-readable to counter --machine-readable
  
  Also cleaned up comments in Darcs.UI.Options.All and removed the (unused) 
  separate humanReadable option.

patch 0309c5ff456ea747626fa55b868158d148dbe5a0
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sun Mar 19 21:15:35 CET 2017
  * add patch index status to show repo command

patch 97973a52bf496657558412562d6fad2ee651b1e0
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Thu Mar 23 10:24:07 CET 2017
  * show repo: removed --files option, removed manual flags parsing
  
  The --files option was abused to enable additional output of number of
  patches and weak hash. This is completely obscure, since --files gives no
  hint at all as to what effect it has here. The two extra lines are now
  printed unconditionally.

patch 5927142501f520ba57b9dab5fb65ebf6da39451d
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Thu Mar 23 10:40:46 CET 2017
  * show repo: fixed excessively borked code indentation

patch 4fa8f800379a580ba43641db41e1ff34f33f5b51
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Thu Mar 23 13:41:12 CET 2017
  * show repo: fixed formatting (boringfile Pref overflowed alignment)

Attachments

• patch-preview.txt
• remove-files-and-directories-options-from-commands-that-don_t-use-them.dpatch
• unnamed

Indeed the --files flag was abused but it had a purpose: when enabled
it made "darcs show repo" run in constant time, while by default, it
runs in  O(#patches) (precisely to show the number of patches, and
since last year, the weak hash).

Not that I am against removing that option, just saying.

2017-03-23 7:46 GMT-03:00 Ben Franksen <bugs@darcs.net>:
>
> New submission from Ben Franksen <ben.franksen@online.de>:
>
> Various fixes and cleanups (annotate, show xxx, options)
>
> 6 patches for repository http://darcs.net/screened:
>
> patch 5ae1b136e088bbf788982110a0d8818036d004f8
> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
> Date:   Thu Mar 23 09:34:27 CET 2017
>   * remove files and directories options from commands that don't use them
>
> patch ac35dc465641a1ae261fb92b88c84dff2b082b4e
> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
> Date:   Thu Mar 23 11:09:33 CET 2017
>   * annotate/options: allow --human-readable to counter --machine-readable
>
>   Also cleaned up comments in Darcs.UI.Options.All and removed the (unused)
>   separate humanReadable option.
>
> patch 0309c5ff456ea747626fa55b868158d148dbe5a0
> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
> Date:   Sun Mar 19 21:15:35 CET 2017
>   * add patch index status to show repo command
>
> patch 97973a52bf496657558412562d6fad2ee651b1e0
> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
> Date:   Thu Mar 23 10:24:07 CET 2017
>   * show repo: removed --files option, removed manual flags parsing
>
>   The --files option was abused to enable additional output of number of
>   patches and weak hash. This is completely obscure, since --files gives no
>   hint at all as to what effect it has here. The two extra lines are now
>   printed unconditionally.
>
> patch 5927142501f520ba57b9dab5fb65ebf6da39451d
> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
> Date:   Thu Mar 23 10:40:46 CET 2017
>   * show repo: fixed excessively borked code indentation
>
> patch 4fa8f800379a580ba43641db41e1ff34f33f5b51
> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
> Date:   Thu Mar 23 13:41:12 CET 2017
>   * show repo: fixed formatting (boringfile Pref overflowed alignment)
>
> ----------
> files: patch-preview.txt, remove-files-and-directories-options-from-commands-that-don_t-use-them.dpatch, unnamed
> messages: 19391
> nosy: bf
> status: needs-screening
> title: remove files and directories options fro... (and 5 more)
>
> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/patch1544>
> __________________________________
> _______________________________________________
> darcs-devel mailing list
> darcs-devel@darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-devel
>

Oh, you should update the help string of "darcs show repo" then.

2017-03-23 8:06 GMT-03:00 Guillaume Hoffmann <bugs@darcs.net>:
>
> Guillaume Hoffmann <guillaumh@gmail.com> added the comment:
>
> Indeed the --files flag was abused but it had a purpose: when enabled
> it made "darcs show repo" run in constant time, while by default, it
> runs in  O(#patches) (precisely to show the number of patches, and
> since last year, the weak hash).
>
> Not that I am against removing that option, just saying.
>
> 2017-03-23 7:46 GMT-03:00 Ben Franksen <bugs@darcs.net>:
>>
>> New submission from Ben Franksen <ben.franksen@online.de>:
>>
>> Various fixes and cleanups (annotate, show xxx, options)
>>
>> 6 patches for repository http://darcs.net/screened:
>>
>> patch 5ae1b136e088bbf788982110a0d8818036d004f8
>> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
>> Date:   Thu Mar 23 09:34:27 CET 2017
>>   * remove files and directories options from commands that don't use them
>>
>> patch ac35dc465641a1ae261fb92b88c84dff2b082b4e
>> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
>> Date:   Thu Mar 23 11:09:33 CET 2017
>>   * annotate/options: allow --human-readable to counter --machine-readable
>>
>>   Also cleaned up comments in Darcs.UI.Options.All and removed the (unused)
>>   separate humanReadable option.
>>
>> patch 0309c5ff456ea747626fa55b868158d148dbe5a0
>> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
>> Date:   Sun Mar 19 21:15:35 CET 2017
>>   * add patch index status to show repo command
>>
>> patch 97973a52bf496657558412562d6fad2ee651b1e0
>> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
>> Date:   Thu Mar 23 10:24:07 CET 2017
>>   * show repo: removed --files option, removed manual flags parsing
>>
>>   The --files option was abused to enable additional output of number of
>>   patches and weak hash. This is completely obscure, since --files gives no
>>   hint at all as to what effect it has here. The two extra lines are now
>>   printed unconditionally.
>>
>> patch 5927142501f520ba57b9dab5fb65ebf6da39451d
>> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
>> Date:   Thu Mar 23 10:40:46 CET 2017
>>   * show repo: fixed excessively borked code indentation
>>
>> patch 4fa8f800379a580ba43641db41e1ff34f33f5b51
>> Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
>> Date:   Thu Mar 23 13:41:12 CET 2017
>>   * show repo: fixed formatting (boringfile Pref overflowed alignment)
>>
>> ----------
>> files: patch-preview.txt, remove-files-and-directories-options-from-commands-that-don_t-use-them.dpatch, unnamed
>> messages: 19391
>> nosy: bf
>> status: needs-screening
>> title: remove files and directories options fro... (and 5 more)
>>
>> __________________________________
>> Darcs bug tracker <bugs@darcs.net>
>> <http://bugs.darcs.net/patch1544>
>> __________________________________
>> _______________________________________________
>> darcs-devel mailing list
>> darcs-devel@darcs.net
>> http://lists.osuosl.org/mailman/listinfo/darcs-devel
>>
>
> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/patch1544>
> __________________________________

BTW, the O(n) is barely noticeable on my machine, it takes about 0.25
seconds.

1 patch for repository http://darcs.net/screened:

patch b0fd55d23032966a453e0993cb57ff3a194497e8
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sun Mar 26 11:45:25 CEST 2017
  * removed mention of --files from help text for show repo command

Attachments

• patch-preview.txt
• removed-mention-of-__files-from-help-text-for-show-repo-command.dpatch
• unnamed

>  * remove files and directories options from commands that don't use 
them

'--files' and '--directories' are accepted by a number of 'show' 
commands and don't seem to do much - as well as "show index" and "show 
patch-index", they apply to "files" and "pristine". Am I missing 
something or is there just more we could clean up?

> Ganesh Sittampalam <ganesh@earth.li> added the comment:
>>  * remove files and directories options from commands that don't use 
> them
> 
> '--files' and '--directories' are accepted by a number of 'show' 
> commands and don't seem to do much - as well as "show index" and "show 
> patch-index", they apply to "files" and "pristine". Am I missing 
> something or is there just more we could clean up?

For 'show files' these options actually do something interesting (limit
output to files or directories). How useful that is is another question.
I wouldn't be sorry to see them gone for good.

> For 'show files' these options actually do something interesting 
(limit
> output to files or directories). How useful that is is another
> question. I wouldn't be sorry to see them gone for good.

I've tried them, e.g. on the darcs repository, and they don't seem to 
make any difference. I get the same output from 'darcs show files', 
'darcs show files --files' and 'darcs show files --directories'. Am I 
missing something?

>  * show repo: removed --files option, removed manual flags parsing
>  * removed mention of --files from help text for show repo command

Seems fair enough given the O(n) is in practice very fast.

>  * annotate/options: allow --human-readable to counter --machine-
readable

Nice cleanup

>  * add patch index status to show repo command

Makes sense

>  * show repo: fixed formatting (boringfile Pref overflowed alignment)
 
Makes sense

>  * show repo: fixed excessively borked code indentation

Fine, reformatting only.

>   * remove files and directories options from commands that don't use 
them

Fine (though I'm still confused about what remaining commands might use 
them).

Thanks for the review!

Re --files/--directories: The only remaining command that supports these
options is 'show files'. The reason that supplying these options make no
difference is that they are both on (enabled) by default. Try 'darcs
show files --no-files' to see an effect.