On Fri, Nov 06, 2015 at 03:20:31PM +0000, Daniel P. Berrange wrote:
On Fri, Nov 06, 2015 at 04:00:04PM +0100, Martin Kletzander wrote:
On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
>Back in
>
> commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b
> Author: Juerg Haefliger <juerg.haefli...@hp.com>
> Date:   Mon Jan 31 06:42:57 2011 -0500
>
>   tests: handle backspace-newline pairs in test input files
>
>all the test argv files were line wrapped so that the args
>were less than 80 characters.
>
>The way the line wrapping was done turns out to be quite
>undesirable, because it often leaves multiple parameters
>on the same line. If we later need to add or remove
>individual parameters, then it leaves us having to redo
>line wrapping.
>
>This commit changes the line wrapping so that every
>single "-param value" is one its own new line. If the
>"value" is still too long, then we break on ',' or ':'
>or ' ' as needed.
>

What if we fix the syntax-check instead and allow longer than 80
character lines in case they have no space in it, or exactly one space
(to allow --parameter option,option,option,...)?  That would make even
corner cases easier to review, e.g. when you remove or add a parameter
into the long list of parameters.

The alternative that I'm actually thinking is that we make
syntax-check skip line length on all these .args files.

Instead re-run the 'test-wrap-argv.pl' script during
syntax-check, and validate that no changeas are made.

This will guarantee that all our .args files are always
using our ideal formatting / line wrapping rules.

Then I could remove these two special cases and just let
us have "-arg value" lines exceed 80 characters when
they need to


OK, if that doesn't take more time then then simple check, so that the
build time isn't getting longer and longer.

>'make check' and 'make syntax-check' of course still pass after
>applying the big change.
>
>diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
>new file mode 100644
>index 0000000..e8c782b
>--- /dev/null
>+++ b/tests/test-wrap-argv.pl
>@@ -0,0 +1,106 @@
>+#!/usr/bin/perl
>+
>+
>+
>+foreach my $file (@ARGV) {
>+    &rewrap($file);
>+}
>+
>+sub rewrap {
>+    my $file = shift;
>+
>+    # Read the original file
>+    open FILE, "<", $file or die "cannot read $file: $!";
>+    my @lines;
>+    while (<FILE>) {
>+        # If there is a trailing '\' then kill the new line
>+        if (/\\$/) {
>+            chomp;

You could've removed newlines from all lines and rewrap everything so
we're consistent.

NB, some files have multiple commands in them, so we can't blindly
remove all new lines. We have to preserve boundaries between
commands.


Oh, I missed that.

Regards,
Daniel
--
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to