Re: [PATCH 1/3] test-lib: Document short options in t/README

[Ilya Bobyr <ilya.bobyr@gmail.com>]

On 3/25/2014 10:23 AM, Junio C Hamano wrote:
> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> On 3/24/2014 4:39 AM, Ramsay Jones wrote:
>>> On 24/03/14 08:49, Ilya Bobyr wrote:
>>> [...]
>>>> [...]
>>>>  
>>>> ---valgrind=<tool>::
>>>> +-v,--valgrind=<tool>::
>>> The -v short option is taken, above ... :-P
>> Right %)
>> Thanks :)
>> This one starts only with "--va", will fix it.
> Please don't.
>
> In general, when option names can be shortened by taking a unique
> prefix, it is better not to give short form in the documentation at
> all.  There is no guarantee that the short form you happen to pick
> when you document it will continue to be unique forever.  When we
> add another --vasomething option, --va will become ambiguous and one
> of these two things must happen:
>
>  (1) --valgrind and --vasomething are equally useful and often used.
>      Neither will get --va and either --val or --vas needs to be
>      given.
>
>  (2) Because we documented --va as --valgrind, people feel that they
>      are entitled to expect --va will stay forever to be a shorthand
>      for --valgrind and nothing else.  The shortened forms will be
>      between --va (or longer prefix of --valgrind) and --vas (or
>      longer prefix of --vasomething).
>
> We would rather want to see (1), as people new to the system do not
> have to learn that --valgrind can be spelled --va merely by being
> the first to appear, and --vasomething must be spelled --vas because
> it happened to come later.  Longer term, nobody should care how the
> system evolved into the current shape, but (2) will require that to
> understand and remember why one is --va and the other has to be --vas.
>
> We already have this suboptimal (2) situation between "--valgrind"
> and "--verbose" options, but a shorter form "v" that is used for
> "verbose" is so widely understood and used that I think it is an
> acceptable exception.  So
>
>          --verbose::
>         +-v::
>                 Give verbose output from the test
>
> is OK, but "--valgrind can be shortened to --va" is not.

Sure, this is exactly what I meant, but I guess, I was too short
so it created ambiguity =)
I was going to just remove the '-v' from '--valgrind'.

Shortening is a separate issue.  I did not look at it.
I can see that it is also not documented.   At the same time
shortening is entirely consistent at the moment, and does not
work for options that take arguments.

My main intent was to document '-r' :)
As no other short form were documented, I had to fix that issue
first.

If there is decision on how shortening should work for all the
options, maybe I could add a paragraph on that and make existing
options more consistent.

I guess the questions would be, should it possible to use short
forms for options that take arguments?

If so, '--valgrind' becomes impossible to shorten because there
is '--valgrind-only' that is a separate option.  Same for
'--verbose'  and '--verbose-only'.

For convenience here is the relevant switch in the way it is
right now:

    case "$1" in
    -d|--d|--de|--deb|--debu|--debug)
        debug=t; shift ;;
   
-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
        immediate=t; shift ;;
   
-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
        GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
    -r)
        shift; test "$#" -ne 0 || {
            echo 'error: -r requires an argument' >&2;
            exit 1;
        }
        run_list=$1; shift ;;
    --run=*)
        run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
    -h|--h|--he|--hel|--help)
        help=t; shift ;;
    -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
        verbose=t; shift ;;
    --verbose-only=*)
        verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    -q|--q|--qu|--qui|--quie|--quiet)
        # Ignore --quiet under a TAP::Harness. Saying how many tests
        # passed without the ok/not ok details is always an error.
        test -z "$HARNESS_ACTIVE" && quiet=t; shift ;;
    --with-dashes)
        with_dashes=t; shift ;;
    --no-color)
        color=; shift ;;
    --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
        valgrind=memcheck
        shift ;;
    --valgrind=*)
        valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    --valgrind-only=*)
        valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    --tee)
        shift ;; # was handled already
    --root=*)
        root=$(expr "z$1" : 'z[^=]*=\(.*\)')
        shift ;;
    *)
        echo "error: unknown test option '$1'" >&2; exit 1 ;;
    esac


P.S.  Sorry it takes me this long to reply.  I will try to be
more responsive, should there will be a discussion :)