Re: [libvirt] [PATCH] tests: redo test argv file line wrapping

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.haefliger(a)hp.com&gt; > > 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

> >'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.

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 :|