You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Nathan Hartman <ha...@gmail.com> on 2019/12/01 16:49:27 UTC
[PATCH] Fix wrapping of 'svn help' output at 80 chars
I noticed that the first line of output of 'svn help' doesn't fit in a
terminal width of 80 characters for:
svn help changelist
svn help cleanup
svn help switch
Even though the strings do fit in 80 characters, this happens because
the name of the command (and any aliases) are prepended to the first
line, making it longer than apparent in the code.
So the following string:
"Either recover from an interrupted operation that left the working copy
locked,\n"
"or remove unwanted files.\n"
Displays like this when 'svn help cleanup' is run in a terminal 80
chars wide:
cleanup: Either recover from an interrupted operation that left the working
copy
locked,
or remove unwanted files.
Patch below...
I didn't commit this yet because it affects translations. When
modifying strings, do I need to modify the po files in the same
commit?
[[[
Index: subversion/svn/svn.c
===================================================================
--- subversion/svn/svn.c (revision 1870686)
+++ subversion/svn/svn.c (working copy)
@@ -636,7 +636,8 @@
{'r', opt_ignore_keywords} },
{ "changelist", svn_cl__changelist, {"cl"}, {N_(
- "Associate (or dissociate) changelist CLNAME with the named files.\n"
+ "Associate (or dissociate) changelist CLNAME with the named\n"
+ "files.\n"
"usage: 1. changelist CLNAME PATH...\n"
" 2. changelist --remove PATH...\n"
)},
@@ -672,8 +673,8 @@
{{'N', N_("obsolete; same as --depth=files")}} },
{ "cleanup", svn_cl__cleanup, {0}, {N_(
- "Either recover from an interrupted operation that left the working
copy locked,\n"
- "or remove unwanted files.\n"
+ "Either recover from an interrupted operation that left the working
copy\n"
+ "locked, or remove unwanted files.\n"
"usage: 1. cleanup [WCPATH...]\n"
" 2. cleanup --remove-unversioned [WCPATH...]\n"
" cleanup --remove-ignored [WCPATH...]\n"
@@ -1875,7 +1876,8 @@
{'N', N_("obsolete; same as --depth=immediates")}} },
{ "switch", svn_cl__switch, {"sw"}, {N_(
- "Update the working copy to a different URL within the same
repository.\n"
+ "Update the working copy to a different URL within the same\n"
+ "repository.\n"
"usage: 1. switch URL[@PEGREV] [PATH]\n"
" 2. switch --relocate FROM-PREFIX TO-PREFIX [PATH...]\n"
"\n"), N_(
]]]
Nathan
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Mon, Dec 02, 2019 at 03:50:14 +0000:
> Instead of running the entire test suite, you can do «cd …/cmdline &&
> ./getopt_tests.py». Run it with --help for details.
By the way, the --help output doesn't document this, but in the syntax
«./getopt_tests.py [<test>]», <test> can be either the number of a test
(shown by --list and by PASS/FAIL messages) or the name of the Python function.
Cheers,
Daniel
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Wed, 04 Dec 2019 23:04 +00:00:
> On Wed, Dec 4, 2019 at 4:05 PM Nathan Hartman <ha...@gmail.com> wrote:
> > On Wed, Dec 4, 2019 at 3:51 AM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > I can confirm that with the patch applied, getopt_tests.py runs and
> > all its tests pass when invoked in all three of the following ways:
> > * 'make check'
> > * 'make check TESTS=subversion/tests/cmdline/getopt_tests.py'
> > * 'cd subversion/tests/cmdline' and './getopt_tests.py'
> >
> > I'll go ahead and commit it... We'll see how the buildbots feel about it. :-)
>
> Committed in r1870854.
Thanks!
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Dec 4, 2019 at 4:05 PM Nathan Hartman <ha...@gmail.com> wrote:
> On Wed, Dec 4, 2019 at 3:51 AM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> I can confirm that with the patch applied, getopt_tests.py runs and
> all its tests pass when invoked in all three of the following ways:
> * 'make check'
> * 'make check TESTS=subversion/tests/cmdline/getopt_tests.py'
> * 'cd subversion/tests/cmdline' and './getopt_tests.py'
>
> I'll go ahead and commit it... We'll see how the buildbots feel about it. :-)
Committed in r1870854.
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Dec 4, 2019 at 3:51 AM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Nathan Hartman wrote on Wed, 04 Dec 2019 05:43 +00:00:
> > I haven't confirmed this yet but I think that fixes it because at some
> > point between initialization and this test running, sys.argv[0] is
> > being modified.
>
> Confirmed.
Thanks. More below...
> > I found two instances of sys.argv[0] being assigned in
> > subversion/tests/cmdline/svntest/main.py -- see _internal_run_tests()
> > and execute_tests(). Not sure if that's where it's happening.
>
> Sounds plausible. The comments next to those assignments support
> this theory.
>
> > I'm running the full test suite now...
> >
> > If all goes well then I think it should be committed.
>
> We can certainly commit it if your test run passes.
I can confirm that with the patch applied, getopt_tests.py runs and
all its tests pass when invoked in all three of the following ways:
* 'make check'
* 'make check TESTS=subversion/tests/cmdline/getopt_tests.py'
* 'cd subversion/tests/cmdline' and './getopt_tests.py'
I'll go ahead and commit it... We'll see how the buildbots feel about it. :-)
More below...
> I think it will be even better for *_tests.py to use __file__ rather
> than rely on sys.argv[0] to be correct, especially if the sys.argv[0]
> fixup happens only after getopt_tests.py's top-level code is run. (We
> can commit the patch I posted first and make a change along these lines
> afterwards.)
That is a bigger change, so I think I'll commit the patch as-is and
then go from there...
Cheers,
Nathan
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Wed, 04 Dec 2019 05:43 +00:00:
> I haven't confirmed this yet but I think that fixes it because at some
> point between initialization and this test running, sys.argv[0] is
> being modified.
Confirmed.
> I found two instances of sys.argv[0] being assigned in
> subversion/tests/cmdline/svntest/main.py -- see _internal_run_tests()
> and execute_tests(). Not sure if that's where it's happening.
Sounds plausible. The comments next to those assignments support
this theory.
> I'm running the full test suite now...
>
> If all goes well then I think it should be committed.
We can certainly commit it if your test run passes.
I think it will be even better for *_tests.py to use __file__ rather
than rely on sys.argv[0] to be correct, especially if the sys.argv[0]
fixup happens only after getopt_tests.py's top-level code is run. (We
can commit the patch I posted first and make a change along these lines
afterwards.)
Cheers,
Daniel
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Dec 1, 2019 at 10:50 PM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Nathan Hartman wrote on Sun, Dec 01, 2019 at 22:22:26 -0500:
> > On a related note, I cannot seem to run *just* getopt_tests.py. If I
> > run it like this, I get failure:
(snip)
> > $ make check TESTS=subversion/tests/cmdline/getopt_tests.py
(snip)
> > However, if I just run 'make check' and wait patiently,
> > getopt_tests.py passes.
> The attached patch fixes it, but I don't know why. I came up with it by
> comparing the handling of getopt_tests_data to other *_tests_data directories.
You're right. Your patch does fix it. (At least for the use case I
mentioned previously -- 'make check TESTS=...')
Your patch moves the following into load_expected_output():
# This directory contains all the expected output from svn.
getopt_output_dir = os.path.join(os.path.dirname(sys.argv[0]),
'getopt_tests_data')
I haven't confirmed this yet but I think that fixes it because at some
point between initialization and this test running, sys.argv[0] is
being modified. I found two instances of sys.argv[0] being assigned in
subversion/tests/cmdline/svntest/main.py -- see _internal_run_tests()
and execute_tests(). Not sure if that's where it's happening.
But I have confirmed that after applying this patch I can successfully
run:
$ make check TESTS=subversion/tests/cmdline/getopt_tests.py
I'm running the full test suite now...
If all goes well then I think it should be committed.
Nathan
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Sun, Dec 01, 2019 at 22:22:26 -0500:
> On a related note, I cannot seem to run *just* getopt_tests.py. If I
> run it like this, I get failure:
>
> [[[
>
> $ make check TESTS=subversion/tests/cmdline/getopt_tests.py
⋮
> tests.log contains (only the first failure shown, others are similar):
You can consult fails.log too. Unfortunately, it's created only right before
'make check' exits, so you can't consult it while the tests are still running.
> [[[
> START: getopt_tests.py
> W: CWD: /Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline
> Traceback (most recent call last):
> File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/svntest/main.py",
> line 1931, in run
> rc = self.pred.run(sandbox)
> File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/svntest/testcase.py",
> line 178, in run
> result = self.func(sandbox)
> File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/getopt_tests.py",
> line 196, in getopt_no_args
> run_one_test(sbox, 'svn')
> File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/getopt_tests.py",
> line 171, in run_one_test
> exp_stdout, exp_stderr = load_expected_output(basename)
> File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/getopt_tests.py",
> line 57, in load_expected_output
> exp_stdout = open(stdout_filename, 'r').readlines()
> IOError: [Errno 2] No such file or directory:
> './build/getopt_tests_data/svn_stdout'
> FAIL: getopt_tests.py 1: run svn with no arguments
> ]]]
>
> So it's looking for build/getopt_tests_data instead of
> subversion/tests/cmdline/getopt_tests_data. I have not figured out why
> yet.
The attached patch fixes it, but I don't know why. I came up with it by
comparing the handling of getopt_tests_data to other *_tests_data directories.
> However, if I just run 'make check' and wait patiently,
> getopt_tests.py passes.
Instead of running the entire test suite, you can do «cd …/cmdline &&
./getopt_tests.py». Run it with --help for details. You can also do, say,
«./davautocheck.sh getopt» (see comments in the script for more details).
Cheers,
Daniel
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Dec 1, 2019 at 1:44 PM Nathan Hartman <ha...@gmail.com> wrote:
> On Sun, Dec 1, 2019 at 12:17 PM Julian Foad <ju...@foad.me.uk> wrote:
> > Nathan Hartman wrote:
> > > I didn't commit this yet because it affects translations. When
> > > modifying strings, do I need to modify the po files in the same
> > > commit?
> >
> > No. Commit first. Translations are updated separately.
>
> Thanks.
>
> Committed in r1870689.
And that promptly caused a test failure in getopt_tests.py, because
that test compares the output of 'svn help log switch' to expected
output, and I rewrapped the first line of help text for 'svn switch'.
Committed r1870698 to adjust the expected output accordingly.
The other 2 rewrapped lines do not appear to affect tests.
On a related note, I cannot seem to run *just* getopt_tests.py. If I
run it like this, I get failure:
[[[
$ make check TESTS=subversion/tests/cmdline/getopt_tests.py
[1/1] getopt_tests.py....................................................FAILURE
At least one test FAILED, checking /Users/nate/ramdrive/svn-trunk/tests.log
FAIL: getopt_tests.py 1: run svn with no arguments
FAIL: getopt_tests.py 2: run svn --version
FAIL: getopt_tests.py 3: run svn --version --quiet
FAIL: getopt_tests.py 4: run svn --version --verbose
FAIL: getopt_tests.py 5: run svn --help
FAIL: getopt_tests.py 6: run svn help
FAIL: getopt_tests.py 7: run svn help bogus-cmd
FAIL: getopt_tests.py 8: run svn help log switch
Summary of test results:
1 test PASSED
8 tests FAILED
Python version: 2.7.16.
SUMMARY: Some tests failed
]]]
tests.log contains (only the first failure shown, others are similar):
[[[
START: getopt_tests.py
W: CWD: /Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline
Traceback (most recent call last):
File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 1931, in run
rc = self.pred.run(sandbox)
File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/svntest/testcase.py",
line 178, in run
result = self.func(sandbox)
File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/getopt_tests.py",
line 196, in getopt_no_args
run_one_test(sbox, 'svn')
File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/getopt_tests.py",
line 171, in run_one_test
exp_stdout, exp_stderr = load_expected_output(basename)
File "/Users/nate/ramdrive/svn-trunk/subversion/tests/cmdline/getopt_tests.py",
line 57, in load_expected_output
exp_stdout = open(stdout_filename, 'r').readlines()
IOError: [Errno 2] No such file or directory:
'./build/getopt_tests_data/svn_stdout'
FAIL: getopt_tests.py 1: run svn with no arguments
]]]
So it's looking for build/getopt_tests_data instead of
subversion/tests/cmdline/getopt_tests_data. I have not figured out why
yet.
However, if I just run 'make check' and wait patiently,
getopt_tests.py passes.
While on the subject of testing, [023/120] fs-test is failing when the
entire test suite runs. I have not investigated that yet. No idea why
it passes on the buildbots and fails on my machine.
Everything mentioned above happened on macOS.
I didn't try on Linux yet.
All other tests pass.
Nathan
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Dec 1, 2019 at 12:17 PM Julian Foad <ju...@foad.me.uk> wrote:
>
>
> Nathan Hartman wrote:
> > I didn't commit this yet because it affects translations. When modifying strings, do I need to modify the po files in the same commit?
>
> No. Commit first. Translations are updated separately.
Thanks.
Committed in r1870689.
Nathan
Re: [PATCH] Fix wrapping of 'svn help' output at 80 chars
Posted by Julian Foad <ju...@foad.me.uk>.
Nathan Hartman wrote:
> I didn't commit this yet because it affects translations. When modifying strings, do I need to modify the po files in the same commit?
No. Commit first. Translations are updated separately.
- Julian