On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
Back in commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b Author: Juerg Haefliger <juerg.haefliger@xxxxxx> 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.
This means that when we come to add / remove parameters from the test files line, the patch diffs will only ever show a single line added/removed which will greatly simplify review work. Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> --- .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 10 +- .../networkxml2firewalldata/nat-default-linux.args | 137 ++++++++++++++++----- .../qemuxml2argv-aarch64-aavmf-virtio-mmio.args | 33 +++-- tests/test-wrap-argv.pl | 106 ++++++++++++++++ 4 files changed, 247 insertions(+), 39 deletions(-) create mode 100644 tests/test-wrap-argv.pl To avoid sending a 10 MB email, I'v only include these 3 example file changes. To view *all* the changes see my staging branch https://gitlab.com/berrange/libvirt/commit/4fe45c39dae176ae4dbc7e126d2a2ff29b49eceb
Could you please remove the maintenance branches so I can add it for the future as a remote without having multiple sets of remote maintenance branches? Simple script, assuming "gitlab" is the name of your remote, is provided below for your convenience: git push gitlab $(git branch -a | \ sed -n '/^[^a-z]*remotes\/gitlab.*[0-9]-maint$/s_.*remotes/gitlab/_:_p')
'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. It looks like this works, But I would like to know your opinion on the suggestion above. If you disagree with that, then ACK to both patches, but let me know what you think, please. Have a nice weekend Martin
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list