Re: [PATCH 1/3] test-lib: Document short options in t/README
On 4/23/2014 11:24 AM, Junio C Hamano wrote: Ilya Bobyr ilya.bo...@gmail.com writes: Most arguments that could be provided to a test have short forms. Unless documented, the only way to learn them is to read the code. Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com --- t/README |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/README b/t/README index caeeb9d..6b93aca 100644 --- a/t/README +++ b/t/README @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate (or -i) command line argument to the test, or by setting GIT_TEST_OPTS appropriately before running make. ---verbose:: +-v,--verbose:: This makes the test more verbose. Specifically, the command being run and their output if any are also output. I was debating myself if the result should look more like this: -v:: --verbose:: This makes the test more verbose. Specifically, the command being run and their output if any are also output. As a straight text file, your version is certainly a lot easier to read, but at the same time, the entire file is written in more or less AsciiDoc format (the list of prerequisites and the list of harness library functions need to be converted to the item:: form for the text to format well, though) and I've seen some efforts by others to run text files in Documentation/ that were originally meant to be consumed as straight text thru AsciiDoc, so the latter form might be a small step for futureproofing. My conclusion at this point is that the original is good for the current need of the project; if somebody wants to include this file from somewhere in Documentation/technical, a conversion to use multiple item1::newlineitem2::newlinedescription headers can be done by that person as part of the make it fully AsciiDoc effort. Thanks. I've changed it. It is a trivial change and it does not seem to be that bad in plain text form either. I do not know the AsciiDoc conventions as have not read its spec. If there are any other conventions I am breaking - let me know. I will read the spec if I will be contributing more to the documentation. P.S. Sorry it takes me this long to reply %) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] test-lib: Document short options in t/README
Ilya Bobyr ilya.bo...@gmail.com writes: Most arguments that could be provided to a test have short forms. Unless documented, the only way to learn them is to read the code. Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com --- t/README |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/README b/t/README index caeeb9d..6b93aca 100644 --- a/t/README +++ b/t/README @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate (or -i) command line argument to the test, or by setting GIT_TEST_OPTS appropriately before running make. ---verbose:: +-v,--verbose:: This makes the test more verbose. Specifically, the command being run and their output if any are also output. I was debating myself if the result should look more like this: -v:: --verbose:: This makes the test more verbose. Specifically, the command being run and their output if any are also output. As a straight text file, your version is certainly a lot easier to read, but at the same time, the entire file is written in more or less AsciiDoc format (the list of prerequisites and the list of harness library functions need to be converted to the item:: form for the text to format well, though) and I've seen some efforts by others to run text files in Documentation/ that were originally meant to be consumed as straight text thru AsciiDoc, so the latter form might be a small step for futureproofing. My conclusion at this point is that the original is good for the current need of the project; if somebody wants to include this file from somewhere in Documentation/technical, a conversion to use multiple item1::newlineitem2::newlinedescription headers can be done by that person as part of the make it fully AsciiDoc effort. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] test-lib: Document short options in t/README
On 3/25/2014 10:23 AM, Junio C Hamano wrote:
Ilya Bobyr ilya.bo...@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 :)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] test-lib: Document short options in t/README
Ilya Bobyr ilya.bo...@gmail.com writes:
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.
We should strive to make the following from gitcli.txt apply
throughout the system:
* many commands allow a long option `--option` to be abbreviated
only to their unique prefix (e.g. if there is no other option
whose name begins with `opt`, you may be able to spell `--opt` to
invoke the `--option` flag), but you should fully spell them out
when writing your scripts; later versions of Git may introduce a
new option whose name shares the same prefix, e.g. `--optimize`,
to make a short prefix that used to be unique no longer unique.
If so, '--valgrind' becomes impossible to shorten because there
is '--valgrind-only' that is a separate option. Same for
'--verbose' and '--verbose-only'.
Correct. If you really cared, --valgrind={yes,no,only} would be (or
have been) a better possibility, though.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] test-lib: Document short options in t/README
Ilya Bobyr ilya.bo...@gmail.com writes: On 3/24/2014 4:39 AM, Ramsay Jones wrote: On 24/03/14 08:49, Ilya Bobyr wrote: Most arguments that could be provided to a test have short forms. Unless documented the only way to learn then is to read the code. Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com --- t/README | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/README b/t/README index caeeb9d..ccb5989 100644 --- a/t/README +++ b/t/README @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate (or -i) command line argument to the test, or by setting GIT_TEST_OPTS appropriately before running make. ---verbose:: +-v,--verbose:: OK [...] ---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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] test-lib: Document short options in t/README
On 24/03/14 08:49, Ilya Bobyr wrote: Most arguments that could be provided to a test have short forms. Unless documented the only way to learn then is to read the code. Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com --- t/README | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/README b/t/README index caeeb9d..ccb5989 100644 --- a/t/README +++ b/t/README @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate (or -i) command line argument to the test, or by setting GIT_TEST_OPTS appropriately before running make. ---verbose:: +-v,--verbose:: OK This makes the test more verbose. Specifically, the command being run and their output if any are also output. @@ -81,7 +81,7 @@ appropriately before running make. numbers matching pattern. The number matched against is simply the running count of the test within the file. ---debug:: +-d,--debug:: This may help the person who is developing a new test. It causes the command defined with test_debug to run. The trash directory (used to store all temporary data @@ -89,18 +89,18 @@ appropriately before running make. failed tests so that you can inspect its contents after the test finished. ---immediate:: +-i,--immediate:: This causes the test to immediately exit upon the first failed test. Cleanup commands requested with test_when_finished are not executed if the test failed, in order to keep the state for inspection by the tester to diagnose the bug. ---long-tests:: +-l,--long-tests:: This causes additional long-running tests to be run (where available), for more exhaustive testing. ---valgrind=tool:: +-v,--valgrind=tool:: The -v short option is taken, above ... :-P Execute all Git binaries under valgrind tool tool and exit with status 126 on errors (just like regular tests, this will only stop the test script when running under -i). ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] test-lib: Document short options in t/README
On 3/24/2014 4:39 AM, Ramsay Jones wrote: On 24/03/14 08:49, Ilya Bobyr wrote: Most arguments that could be provided to a test have short forms. Unless documented the only way to learn then is to read the code. Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com --- t/README | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/README b/t/README index caeeb9d..ccb5989 100644 --- a/t/README +++ b/t/README @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate (or -i) command line argument to the test, or by setting GIT_TEST_OPTS appropriately before running make. ---verbose:: +-v,--verbose:: OK [...] ---valgrind=tool:: +-v,--valgrind=tool:: The -v short option is taken, above ... :-P Right %) Thanks :) This one starts only with --va, will fix it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
