Hello, I've put together an experimental patch that aims to copy whitespace from previous lines when creating new nodes. Generally this is just to make files prettier, but we saw in the Cobbler/YAML case it's required as whitespace affects parsing. It works by changing the definition of "del" lenses (del RE STR) when they're used in the "create" path.
When a del lens is processed in the put direction, it now stores the last string that it restored (matched by RE). If the lens is then used to create, this string is restored instead of the default string given by STR. The default string STR is only used when no previous put has occurred. Since del is used for whitespace and any other characters that don't make it into the tree, this has the effect that they're "copied" from the last occurrence that del lens, so the file looks more consistent. This mostly seems to work in practice. The main problem with it is that lenses - especially for whitespace - are reused for many different fields in a file. If for example Util.del_ws was used for /etc/fstab, the same del lens would be used five times for different field widths throughout a single line. If a new entry was created, this patch would start restoring the width from the last deleted section on the last row, rather than the first deleted section. The change to lenses/tests/test_cgrules.aug shows how multiple fields aren't copied correctly. In tests/test-augtool/ini-yum-newsec.sh you also see where a del lens is used for the whitespace around key/value separators and again for the whitespace preceding comments - making it less consistent now than it was. To fix this would mean duplicating a lot of del lenses, one for each use. Perhaps a better way would be to create a set of registers (like vim, or counter/seq) where deleted strings are stored and optionally assigned a name so the right one can be restored. This would mean a new lens primitive, or addition to "del", so many changes would be needed to existing file lenses to use the functionality. Thoughts welcomed. Thanks, -- Dominic Cleal Red Hat Consulting m: +44 (0)7817 878113
>From cd2a32688a3b1ea016660ac1bb3f06886af2f4a4 Mon Sep 17 00:00:00 2001 From: Dominic Cleal <[email protected]> Date: Sun, 8 Apr 2012 13:26:21 +0100 Subject: [PATCH] Re-use last deleted string when del lens creates new nodes This has the effect of "copying" indentation and other file features when new nodes are created. * src/put.c: for L_DEL lenses, store the last deleted string in the lens state and use it instead of the supplied STR when creating new nodes. * tests/: updated tests to expect new 'del' behaviour. Not all of them make sense, because del lenses for whitespace are often reused for multiple fields. --- lenses/tests/test_access.aug | 2 +- lenses/tests/test_cgrules.aug | 2 +- lenses/tests/test_cobblermodules.aug | 4 +- lenses/tests/test_debctrl.aug | 2 +- lenses/tests/test_device_map.aug | 2 +- lenses/tests/test_logrotate.aug | 4 +- lenses/tests/test_odbc.aug | 2 +- lenses/tests/test_properties.aug | 2 +- lenses/tests/test_shellvars_list.aug | 4 +- lenses/tests/test_syslog.aug | 4 +- lenses/tests/test_yum.aug | 2 +- src/lens.h | 3 +- src/put.c | 53 +++++++++++++++++++++++++-- tests/modules/pass_array.aug | 2 +- tests/modules/pass_del_create_copy.aug | 25 +++++++++++++ tests/test-augtool/ini-yum-newkey.sh | 2 +- tests/test-augtool/ini-yum-newsec.sh | 2 +- tests/test-augtool/rec-append-record.sh | 2 +- tests/test-augtool/rec-hosts-reorder-new.sh | 2 +- 19 files changed, 96 insertions(+), 25 deletions(-) create mode 100644 tests/modules/pass_del_create_copy.aug diff --git a/lenses/tests/test_access.aug b/lenses/tests/test_access.aug index fcb63df..f5e6b36 100644 --- a/lenses/tests/test_access.aug +++ b/lenses/tests/test_access.aug @@ -83,7 +83,7 @@ test Access.lns put conf after + : ALL EXCEPT john @wheel : ALL EXCEPT LOCAL .win.tue.nl # No spaces +:root:.example.com -- : ALL : ALL +-:ALL:ALL " (* Test: Access.lns diff --git a/lenses/tests/test_cgrules.aug b/lenses/tests/test_cgrules.aug index 3e29d4f..0d00704 100644 --- a/lenses/tests/test_cgrules.aug +++ b/lenses/tests/test_cgrules.aug @@ -28,5 +28,5 @@ test Cgrules.lns put conf after = "#cgrules test configuration file poooeter cpu test3/ % memory memtest/ -% devices newtest/ +% devices newtest/ " diff --git a/lenses/tests/test_cobblermodules.aug b/lenses/tests/test_cobblermodules.aug index c525f6c..08b8a46 100644 --- a/lenses/tests/test_cobblermodules.aug +++ b/lenses/tests/test_cobblermodules.aug @@ -23,8 +23,8 @@ modules = auth_denyall [serializes] settings = serializer_catalog -distro=serializer_catalog -repo=serializer_catalog +distro = serializer_catalog +repo = serializer_catalog [authentication] modules = auth_denyall " diff --git a/lenses/tests/test_debctrl.aug b/lenses/tests/test_debctrl.aug index f81d1e2..c78f400 100644 --- a/lenses/tests/test_debctrl.aug +++ b/lenses/tests/test_debctrl.aug @@ -158,7 +158,7 @@ test Debctrl.src_entries after set "/Uploaders/4" "baz@bar" = "Uploaders: foo@bar, Dominique Dumont <[email protected]>,\n" . " gregor herrmann <[email protected]>,\n" - . " baz@bar\n" + . " baz@bar\n" test Debctrl.lns put (source."\nPackage: test\nDescription: foobar\n") after diff --git a/lenses/tests/test_device_map.aug b/lenses/tests/test_device_map.aug index 2a7810a..0a12ce4 100644 --- a/lenses/tests/test_device_map.aug +++ b/lenses/tests/test_device_map.aug @@ -30,5 +30,5 @@ module Test_device_map = (hd0,a) /dev/sda1 (0x80) /dev/sda (128) /dev/sda -(hd2,1)\t/dev/sdb1 +(hd2,1) /dev/sdb1 " diff --git a/lenses/tests/test_logrotate.aug b/lenses/tests/test_logrotate.aug index 99aa84d..902f3bc 100644 --- a/lenses/tests/test_logrotate.aug +++ b/lenses/tests/test_logrotate.aug @@ -178,7 +178,7 @@ test Logrotate.lns put "/file {\n size=5M\n}\n" after = "/file { size=5M -\tprerotate + prerotate \tfoobar \tendscript\n}\n" @@ -187,7 +187,7 @@ test Logrotate.lns put "/file {\n size=5M\n}\n" after = "/file { size=5M -\tprerotate + prerotate \tfoobar\n \tendscript\n}\n" diff --git a/lenses/tests/test_odbc.aug b/lenses/tests/test_odbc.aug index 5f886f9..fe24e3e 100644 --- a/lenses/tests/test_odbc.aug +++ b/lenses/tests/test_odbc.aug @@ -63,7 +63,7 @@ FileUsage = 1 # Driver from the MyODBC package # Setup from the unixODBC package Description = ODBC for MySQL -Driver = /usr/lib64/libmyodbc3.so#note the path +Driver = /usr/lib64/libmyodbc3.so # note the path Setup = /usr/lib/libodbcmyS.so FileUsage = 1 " diff --git a/lenses/tests/test_properties.aug b/lenses/tests/test_properties.aug index 594dda6..d58976a 100644 --- a/lenses/tests/test_properties.aug +++ b/lenses/tests/test_properties.aug @@ -29,5 +29,5 @@ test lns put conf after tomcat.port = 99 tomcat.application.name=testapp tomcat.application.description=my test application -tomcat.application.host=foo.network.com + tomcat.application.host=foo.network.com " diff --git a/lenses/tests/test_shellvars_list.aug b/lenses/tests/test_shellvars_list.aug index 5bc09e0..71b7ce3 100644 --- a/lenses/tests/test_shellvars_list.aug +++ b/lenses/tests/test_shellvars_list.aug @@ -32,7 +32,7 @@ LOADER_TYPE=\"grub\" (* append a value *) test Shellvars_list.lns put "VAR=\"test1\t \ntest2\"\n" after set "VAR/value[last()+1]" "test3" - = "VAR=\"test1\t \ntest2 test3\"\n" + = "VAR=\"test1\t \ntest2\t \ntest3\"\n" (* in double quoted lists, single quotes and escaped values are allowed *) test Shellvars_list.lns get "VAR=\"test'1 test2 a\ \\\"longer\\\"\ test\"\n" = @@ -61,7 +61,7 @@ FAILSAVE_APPEND=\"console=ttyS0\" (* leading/trailing) *) test Shellvars_list.lns put "VAR=' \t test1\t \ntest2 '\n" after set "VAR/value[last()+1]" "test3" - = "VAR=' \t test1\t \ntest2 test3 '\n" + = "VAR=' \t test1\t \ntest2\t \ntest3 '\n" (* change quotes (leading/trailing whitespaces are lost *) test Shellvars_list.lns put "VAR=' \t test1\t \ntest2 '\n" after diff --git a/lenses/tests/test_syslog.aug b/lenses/tests/test_syslog.aug index 83eb563..db3e0ab 100644 --- a/lenses/tests/test_syslog.aug +++ b/lenses/tests/test_syslog.aug @@ -225,7 +225,7 @@ daemon.info /var/log/cvsupd.log set "/entry[1]/selector/facility" "daemon" ; set "/entry[1]/selector/level" "info" ; set "/entry[1]/action/file" "/foo" - = "daemon.info /foo\n*.*\t/var\n" + = "daemon.info /foo\n*.* /var\n" (* inserting an entry after *) test Syslog.lns put "*.* /var\n" after @@ -233,7 +233,7 @@ daemon.info /var/log/cvsupd.log set "/entry[2]/selector/facility" "daemon" ; set "/entry[2]/selector/level" "info" ; set "/entry[2]/action/file" "/foo" - = "*.* /var\ndaemon.info\t/foo\n" + = "*.* /var\ndaemon.info /foo\n" (* insert sync on a file *) test Syslog.lns put "*.* /var\n" after diff --git a/lenses/tests/test_yum.aug b/lenses/tests/test_yum.aug index f6a8d82..3d28e54 100644 --- a/lenses/tests/test_yum.aug +++ b/lenses/tests/test_yum.aug @@ -118,7 +118,7 @@ gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-remi test Yum.lns put cont after set "main/gpgcheck" "1" = - cont . "gpgcheck=1\n" + cont . "gpgcheck=1 \n" (* We are actually stricter than yum in checking syntax. The yum.conf *) (* man page mentions that it is illegal to have multiple baseurl keys *) diff --git a/src/lens.h b/src/lens.h index 62c2f77..0411de8 100644 --- a/src/lens.h +++ b/src/lens.h @@ -1,7 +1,7 @@ /* * lens.h: Repreentation of lenses * - * Copyright (C) 2007-2011 David Lutterkort + * Copyright (C) 2007-2012 David Lutterkort * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -95,6 +95,7 @@ struct lens { */ struct regexp *regexp; /* L_STORE, L_KEY */ struct string *string; /* L_VALUE, L_LABEL, L_SEQ, L_COUNTER */ + char *last; /* L_DEL */ }; /* Combinators */ struct lens *child; /* L_SUBTREE, L_STAR, L_MAYBE, L_SQUARE */ diff --git a/src/put.c b/src/put.c index 56732d2..6ba30bb 100644 --- a/src/put.c +++ b/src/put.c @@ -1,7 +1,7 @@ /* * put.c: * - * Copyright (C) 2007-2011 David Lutterkort + * Copyright (C) 2007-2012 David Lutterkort * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -469,10 +469,12 @@ static void put_del(ATTRIBUTE_UNUSED struct lens *lens, struct state *state) { assert(state->skel != NULL); assert(state->skel->tag == L_DEL); if (lens->string != NULL) { - fprintf(state->out, "%s", state->skel->text); + fprintf(state->out, "%s", state->skel->text); + lens->last = state->skel->text; } else { - /* L_DEL with NULL string: replicate the current key */ + /* L_DEL with NULL string: replicate the current key */ fprintf(state->out, "%s", state->key); + lens->last = NULL; } } @@ -661,7 +663,9 @@ static void create_subtree(struct lens *lens, struct state *state) { static void create_del(struct lens *lens, struct state *state) { assert(lens->tag == L_DEL); - if (lens->string != NULL) { + if (lens->last != NULL) { + print_escaped_chars(state->out, lens->last); + } else if (lens->string != NULL) { print_escaped_chars(state->out, lens->string->str); } else { /* L_DEL with NULL string: replicate the current key */ @@ -785,6 +789,46 @@ static void create_lens(struct lens *lens, struct state *state) { } } +/* Clear any state held in the lenses themselves */ +static void clear_lens_state(struct lens *lens) { + switch(lens->tag) { + case L_DEL: + /* Last deleted string, memory owned by state */ + if (lens->last != NULL) { + lens->last = NULL; + } + break; + case L_CONCAT: + case L_UNION: + for (int i=0; i < lens->nchildren; i++) { + clear_lens_state(lens->children[i]); + } + break; + case L_SUBTREE: + case L_STAR: + case L_MAYBE: + case L_SQUARE: + clear_lens_state(lens->child); + break; + case L_REC: + if (lens->rec_internal == 0) { + clear_lens_state(lens->child); + } + break; + case L_STORE: + case L_KEY: + case L_LABEL: + case L_VALUE: + case L_SEQ: + case L_COUNTER: + /* Nothing to do */ + break; + default: + assert(0); + break; + } +} + void lns_put(FILE *out, struct lens *lens, struct tree *tree, const char *text, struct lns_error **err) { struct state state; @@ -811,6 +855,7 @@ void lns_put(FILE *out, struct lens *lens, struct tree *tree, state.key = tree->label; put_lens(lens, &state); + clear_lens_state(lens); free(state.path); free_split(state.split); free_skel(state.skel); diff --git a/tests/modules/pass_array.aug b/tests/modules/pass_array.aug index 7a4da4c..e8a9f1d 100644 --- a/tests/modules/pass_array.aug +++ b/tests/modules/pass_array.aug @@ -14,4 +14,4 @@ module Pass_array = let lns = [ key /[a-z]+/ . del "=" "=" . array ] test lns put "var=(v1 v2)" after - set "var/3" "v3" = "var=(v1 v2\tv3)" + set "var/3" "v3" = "var=(v1 v2 v3)" diff --git a/tests/modules/pass_del_create_copy.aug b/tests/modules/pass_del_create_copy.aug new file mode 100644 index 0000000..2098818 --- /dev/null +++ b/tests/modules/pass_del_create_copy.aug @@ -0,0 +1,25 @@ +(* Test that the del lens will copy previous deleted strings (if available) and + use them when creating new nodes, rather than using the string given *) +module Pass_del_create_copy = + + let indent = del /[ \t]*/ " " + let lns = [ indent . label "entry" . store /[^ \t\n]+/ . del "\n" "\n" ]* + + test lns get "a\n b\n c\n" = + { "entry" = "a" } + { "entry" = "b" } + { "entry" = "c" } + + (* 'c' should copy indent from 'b' *) + test lns put "a\n b\n" after + set "entry[3]" "c" + = "a\n b\n c\n" + + (* 'c' should use the string in the del lens *) + (* When using lns, it's "putting" 'c', 'a' and creating 'b' as they share + the same keys. This test uses different keys. *) + let keytest = [ indent . key /[a-z]+/ . del "\n" "\n" ]* + test keytest put "a\n b\n" after + insb "c" "a" + = " c\na\n b\n" + diff --git a/tests/test-augtool/ini-yum-newkey.sh b/tests/test-augtool/ini-yum-newkey.sh index ee9c432..cede985 100644 --- a/tests/test-augtool/ini-yum-newkey.sh +++ b/tests/test-augtool/ini-yum-newkey.sh @@ -11,4 +11,4 @@ diff='--- /etc/yum.conf # PUT YOUR REPOS HERE OR IN separate files named file.repo # in /etc/yum.repos.d -+newparam=newval' ++newparam = newval' diff --git a/tests/test-augtool/ini-yum-newsec.sh b/tests/test-augtool/ini-yum-newsec.sh index 8060dac..95908d3 100644 --- a/tests/test-augtool/ini-yum-newsec.sh +++ b/tests/test-augtool/ini-yum-newsec.sh @@ -12,4 +12,4 @@ diff='--- /etc/yum.conf # PUT YOUR REPOS HERE OR IN separate files named file.repo # in /etc/yum.repos.d +[other] -+newparam=newval' ++newparam = newval' diff --git a/tests/test-augtool/rec-append-record.sh b/tests/test-augtool/rec-append-record.sh index 976cc66..afe17d8 100644 --- a/tests/test-augtool/rec-append-record.sh +++ b/tests/test-augtool/rec-append-record.sh @@ -15,4 +15,4 @@ diff='--- /etc/pam.d/newrole account include\tsystem-auth password include\tsystem-auth session required\tpam_namespace.so unmnt_remnt no_unmount_on_close -+auth\tinclude\tsystem-auth' ++auth include system-auth' diff --git a/tests/test-augtool/rec-hosts-reorder-new.sh b/tests/test-augtool/rec-hosts-reorder-new.sh index d917a06..0368c6e 100644 --- a/tests/test-augtool/rec-hosts-reorder-new.sh +++ b/tests/test-augtool/rec-hosts-reorder-new.sh @@ -28,4 +28,4 @@ diff='--- /etc/hosts #172.31.122.254 granny.watzmann.net granny puppet #172.31.122.1 galia.watzmann.net galia 172.31.122.14 orange.watzmann.net orange -+127.0.0.1\tlocalhost.localdomain localhost galia.watzmann.net galia' ++127.0.0.1 localhost.localdomain localhost galia.watzmann.net galia' -- 1.7.7.6
_______________________________________________ augeas-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/augeas-devel
