darcs

Issue 1457 Replace --no-foo switch with --foo=no

Title Replace --no-foo switch with --foo=no
Priority feature Status needs-implementation
Milestone Resolved in
Superseder Nosy List btcoburn, dmitry.kurochkin, kowey, markstos, mndrix, mornfall, thorkilnaur, twb
Assigned To
Topics UI

Created on 2009-05-05.03:30:24 by twb, last changed 2012-02-18.19:02:00 by mndrix.

Messages
msg7786 (view) Author: twb Date: 2009-05-05.03:30:19
For some time I have been annoyed by the inconsistency of --foo
vs. --dont-foo or --no-foo in our command-line options.  On the user
list it was felt that this inconsistency was justified because FSVO
foo, "dont[sic] foo" was better English than "no foo".

It occurs to me that this case could be handled better by allowing
options to take arguments:

    Proposed  Current equivalent
    ========  ==================
    --foo     --foo
    --foo=yes --foo
    --foo=no  --no-foo (or --dont-foo)

This would obviate issue1291 (compress option list) because

    --test                          run the test script
    --no-test                       don't run the test script

would become something like

    --test=[YES|no]                 run the test script

This would also obviate the ugly "option sets" that Darcs has
internally, by replacing them with *one* option that can take various
arguments.  The most obvious example for this would be

    --darcs-2
    --hashed
    --old-fashioned-inventory
    [...more, as we add formats]

becoming something like

    --format=[darcs-2|hashed|old-fashioned-inventory]
msg7787 (view) Author: btcoburn Date: 2009-05-05.14:14:48
Sounds good.

Would it make sense to make some of the other option sets more logical at the
same time? 

For example in "darcs changes":
--xml-output
--human-readable

become

--output=xml|human

or maybe better

--ui=xml
--ui=terminal

There are probably some other options that deserve the same attention.
msg7788 (view) Author: twb Date: 2009-05-06.02:57:43
On Tue, May 05, 2009 at 02:14:51PM -0000, Ben Coburn wrote:
> Would it make sense to make some of the other option sets more
> logical at the same time?  For example in "darcs changes":
> --xml-output and --human-readable become --output=xml|human

Yes, exactly.  This is a sweeping UI change.

I would prefer to avoid unnecessary heterogeneity of option names with
respect to other popular VCSs, so in this specific example I would
prefer "darcs changes --style=xml", because that's what hg calls it.
msg7789 (view) Author: kowey Date: 2009-05-06.12:52:34
Looks like the best thing to do is just to implement it and then solicit
opinions on darcs-users.

My first reaction is "ugh, change"; but I also understand that sometimes change
is necessary to reach a more stable position.
msg8473 (view) Author: kowey Date: 2009-08-24.15:32:33
I've merged in issue1557.
I think issue1431 and issue1387 would be helped by this.
msg8842 (view) Author: twb Date: 2009-09-19.04:20:10
This bug may be subsumed by issue1550.  Regarding it, on darcs-users
Neil Mitchell pointed out that the current approach simply passes a
list of allowed options to each command, and relies on each command
knowing what to do about various uncommon combinations.

This results in subtle bugs.  For example, specifying both options
"--no-compress --compress" will reduce to "--no-compress" in MOST
commands, but will reduce to "--compress" in darcs optimize.
msg11774 (view) Author: kowey Date: 2010-07-17.14:09:42
I'm becoming a fan of this proposal (provided we can do it in a not too 
disruptive way).

See this example case using darcs record help.

See notes [!!!] for places where the flag change is relatively large

darcs record help for issue1457

Options:
  -m PATCHNAME  --name=PATCHNAME                         name of patch
  -A EMAIL      --author=EMAIL                           specify author 
id
                --test[=yes/no]                          run the test 
script (default: no)
                --remove-test-directory[=yes/no]         remove the test 
directory (default: yes) [!!!]
  -a            --all                                    answer yes to 
all patches
                --pipe                                   ask user 
interactively for the patch metadata
  -i            --interactive[=yes/no]                   prompt user 
interactively (default: yes)
                --ask-deps[=yes/no]                      ask for extra 
dependencies (default: no)
                --edit-long-comment[=yes/no/prompt]      edit the long 
comment by default (default: prompt) [!!!]
  -l            --look-for-adds[=yes/no]                 look for (non-
boring) files that could be added (default: no)
                --repodir=DIRECTORY                      specify the 
repository directory in which to run
                --disable                                disable this 
command
  -h            --help                                   shows brief 
description of command and its arguments

Advanced options:
                --debug[=yes/no]                         give debug 
output
                --debug-http                             give debug 
output for libcurl
  -v            --verbosity[=0..2]                       verbosity level 
(default: 1) [!!!]
  -q            --quiet                                  suppress 
informational output (same as --verbosity=quiet)
                --timings                                provide 
debugging timings information
                --logfile=FILE                           give patch name 
and comment in file
                --delete-logfile[=yes/no]                delete the 
logfile when done (default: no)
                --compress[=yes/no]                      create 
compressed patches (default: yes)
                --trust-times[=yes/no]                   trust the file 
modification times (default: yes) [!!!]
                --umask=UMASK                            specify umask 
to use when writing
                --set-scripts-executable[=yes/no]        make scripts 
executable (default: no)
                --use-cache[=yes/no]                     use patch 
caches (default: yes) [!!!]
                --posthook=COMMAND                       specify command 
to run after this darcs command
                --run-posthook[=yes/no/prompt]           run posthook 
command (default: prompt)
                --prehook=COMMAND                        specify command 
to run before this darcs command
                --run-prehook[=yes/no/prompt]            run prehook 
command without prompting (default: prompt)

Notes
-----
One potential principle (to be taken with a grain of salt) is that
--foo=[yes/no] flags should (A) use a verb and (B) use the verb that 
most
evokes darcs actively doing something.

So --test-directory[=remove/leave] is potentially confusing because if 
you're
not paying attention, you might think that one should pass in a test 
directory
name to that flag.

Likewise, --leave-test-directory is perhaps suboptimal compared to
--remove-test-directory (because it means you're passing a flag to tell 
darcs
to *not* take an action).

This second half of this principle slightly improves resistance to 
defaults
changing (in other words, if you make the decision on the basis of
--remove-directory being the default and --leave-directory being the 
special
case, then the day you change your mind about the default, the basis for 
your
choice is rendered void).

* remove-test-directory: was --{remove,leave}-test-directory.
  thinking that --test-directory expects a directory
* edit-long-comment: was --{edit,no,prompt}-long-comment
* verbosity: was --verbose, --standard-verbosity, --quiet.  Note that 
debug output
  is considered orthogonal to this (you could have --debug -q).

  Note also that the default for the command would be --verbosity=1 (no 
args),
  but if you just supply --verbosity you get --verbosity=2.  This sort 
of confusion
  makse me think it's best to make the argument a required one.
* trust-times: was --{dont-,no-,}ignore-times; I figured that the 
double-negative
  --ignore-times=no would just confuse people
* use-cache: was --no-cache (with no option to override!)
msg11775 (view) Author: kowey Date: 2010-07-17.14:11:40
On Sat, Jul 17, 2010 at 14:09:43 +0000, Eric Kow wrote:
> See this example case using darcs record help.

Ugh, should have used email instead of the web interface.

>   -m PATCHNAME  --name=PATCHNAME                         name of patch
>   -A EMAIL      --author=EMAIL                           specify author 
> id

See notes [!!!] for places where the flag change is relatively large

darcs record help for issue1457

Options:
  -m PATCHNAME  --name=PATCHNAME                         name of patch
  -A EMAIL      --author=EMAIL                           specify author id
                --test[=yes/no]                          run the test script (default: no)
                --remove-test-directory[=yes/no]         remove the test directory (default: yes) [!!!]
  -a            --all                                    answer yes to all patches
                --pipe                                   ask user interactively for the patch metadata
  -i            --interactive[=yes/no]                   prompt user interactively (default: yes)
                --ask-deps[=yes/no]                      ask for extra dependencies (default: no)
                --edit-long-comment[=yes/no/prompt]      edit the long comment by default (default: prompt) [!!!]
  -l            --look-for-adds[=yes/no]                 look for (non-boring) files that could be added (default: no)
                --repodir=DIRECTORY                      specify the repository directory in which to run
                --disable                                disable this command
  -h            --help                                   shows brief description of command and its arguments

Advanced options:
                --debug[=yes/no]                         give debug output
                --debug-http                             give debug output for libcurl
  -v            --verbosity[=0..2]                       verbosity level (default: 1) [!!!]
  -q            --quiet                                  suppress informational output (same as --verbosity=quiet)
                --timings                                provide debugging timings information
                --logfile=FILE                           give patch name and comment in file
                --delete-logfile[=yes/no]                delete the logfile when done (default: no)
                --compress[=yes/no]                      create compressed patches (default: yes)
                --trust-times[=yes/no]                   trust the file modification times (default: yes) [!!!]
                --umask=UMASK                            specify umask to use when writing
                --set-scripts-executable[=yes/no]        make scripts executable (default: no)
                --use-cache[=yes/no]                     use patch caches (default: yes) [!!!]
                --posthook=COMMAND                       specify command to run after this darcs command
                --run-posthook[=yes/no/prompt]           run posthook command (default: prompt)
                --prehook=COMMAND                        specify command to run before this darcs command
                --run-prehook[=yes/no/prompt]            run prehook command without prompting (default: prompt)

Notes
-----
One potential principle (to be taken with a grain of salt) is that
--foo=[yes/no] flags should (A) use a verb and (B) use the verb that most
evokes darcs actively doing something.

So --test-directory[=remove/leave] is potentially confusing because if you're
not paying attention, you might think that one should pass in a test directory
name to that flag.

Likewise, --leave-test-directory is perhaps suboptimal compared to
--remove-test-directory (because it means you're passing a flag to tell darcs
to *not* take an action).

This second half of this principle slightly improves resistance to defaults
changing (in other words, if you make the decision on the basis of
--remove-directory being the default and --leave-directory being the special
case, then the day you change your mind about the default, the basis for your
choice is rendered void).

* remove-test-directory: was --{remove,leave}-test-directory.
  thinking that --test-directory expects a directory
* edit-long-comment: was --{edit,no,prompt}-long-comment
* verbosity: was --verbose, --standard-verbosity, --quiet.  Note that debug output
  is considered orthogonal to this (you could have --debug -q).

  Note also that the default for the command would be --verbosity=1 (no args),
  but if you just supply --verbosity you get --verbosity=2.  This sort of confusion
  makse me think it's best to make the argument a required one.
* trust-times: was --{dont-,no-,}ignore-times; I figured that the double-negative
  --ignore-times=no would just confuse people
* use-cache: was --no-cache (with no option to override!)

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, please try +44 (0)1273 64 2905.
msg11802 (view) Author: markstos Date: 2010-07-19.13:50:49
I support this change and agree it could replace issue1291.
History
Date User Action Args
2009-05-05 03:30:25twbcreate
2009-05-05 14:14:51btcoburnsetstatus: unread -> unknown
nosy: + btcoburn
messages: + msg7787
2009-05-06 02:57:46twbsetnosy: kowey, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7788
2009-05-06 12:52:38koweysetpriority: feature
status: unknown -> needs-reproduction
messages: + msg7789
nosy: kowey, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
2009-08-24 15:29:54koweylinkissue1557 superseder
2009-08-24 15:30:37koweysetstatus: needs-reproduction -> needs-implementation
nosy: kowey, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
2009-08-24 15:32:36koweysettopic: + UI
nosy: kowey, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg8473
2009-08-25 17:44:54adminsetnosy: + darcs-devel, - simon
2009-08-27 14:32:18adminsetnosy: kowey, darcs-devel, twb, thorkilnaur, btcoburn, dmitry.kurochkin
2009-08-27 17:26:28koweylinkissue1431 superseder
2009-09-19 04:20:20twbsetnosy: kowey, darcs-devel, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg8842
2010-07-17 14:09:43koweysetnosy: + mornfall, - darcs-devel
messages: + msg11774
2010-07-17 14:11:41koweysetmessages: + msg11775
2010-07-19 13:50:50markstossetnosy: + markstos
messages: + msg11802
2010-12-16 09:43:49koweysettitle: Replace --no-foo with --foo=no -> Replace --no-foo switch with --foo=no
2011-02-17 12:02:37koweylinkissue2039 superseder
2012-02-18 19:02:00mndrixsetnosy: + mndrix