scripts/config: use sed's POSIX interface
PATCH v2 1/1 scripts/config: use sed's POSIX interface
Script config' relies on extensions of GNU sed', and is thus not working on all Unixes:
- in-place edition of files (-i), which can be replaced with a temporary file;
- extended-regexps (-r), which can be split into basic regexps;
- single-line calls to `a' command, while some implementations require a leading newline before the parameter.
Rewrite calls to `sed' to comply with POSIX interface, and move them to helper functions.
---
Changes in v2:
- ANSI C style quoting to produce newlines ($'\n') is replaced with printf's, not to introduce further dependency on bash.
- helper functions are introduced to wrap calls to `sed'.
scripts/config | 41 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-)
scripts/config +38 -3
@@ -60,15 +60,48 @@ checkarg() { fi } +txt_append() { + local anchor="$1" + local insert="$2" + local infile="$3" + local outfile="$4" + + # sed append cmd: 'a\' + newline + text + newline + cmd="$(printf "a\\%b$insert%b" "\n" "\n")" + + sed -e "/$anchor/$cmd" "$infile" >"$outfile" +} + +txt_subst() { + local before="$1" + local after="$2" + local infile="$3" + local outfile="$4" + + sed -e "s/$before/$after/" "$infile" >"$outfile" +} + +txt_delete() { + local text="$1" + local infile="$2" + local outfile="$3" + + sed -e "/$text/d" "$infile" >"$outfile" +} + set_var() { local name=$1 new=$2 before=$3 name_re="^($name=|# $name is not set)" before_re="^($before=|# $before is not set)" if test -n "$before" && grep -Eq "$before_re" "$FN"; then - sed -ri "/$before_re/a $new" "$FN" + txt_append "^$before=" "$new" "$FN" "$FN.swp" + txt_append "^# $before is not set" "$new" "$FN.swp" "$FN" + rm "$FN.swp" elif grep -Eq "$name_re" "$FN"; then - sed -ri "s:$name_re.*:$new:" "$FN" + txt_subst "^$name=.*" "$new" "$FN" "$FN.swp" + txt_subst "^# $name is not set" "$new" "$FN.swp" "$FN" + rm "$FN.swp" else echo "$new" >>"$FN" fi
@@ -77,7 +110,9 @@ set_var() { undef_var() { local name=$1 - sed -ri "/^($name=|# $name is not set)/d" "$FN" + txt_delete "^$name=" "$FN" "$FN.swp" + txt_delete "^# $name is not set" "$FN.swp" "$FN" + rm "$FN.swp" } if [ "$1" = "--file" ]; then
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Clément, All,
On 2013-07-13 00:26 +0200, Clement Chauplannaz spake thusly:
Quoted 7 lines
Script
config' relies on extensions ofGNU sed', and is thus not working on all Unixes:
- in-place edition of files (-i), which can be replaced with a temporary file;
- extended-regexps (-r), which can be split into basic regexps;
- single-line calls to `a' command, while some implementations require a leading newline before the parameter.
Just nit-picking on the commit log: can you indent the bullet-list, and the continuation lines, so it is easier to read, like:
- in-place edition of files (-i), which can be replaced with
a temporary file;
Quoted 4 lines
Rewrite calls to `sed' to comply with POSIX interface, and move them to helper functions.
Signed-off-by Clement Chauplannaz <[email protected]>
[--SNIP--]
scripts/config checkarg() { · line 70
Quoted 8 lines
scripts/config +8
-0+txt_append() { + local anchor="$1" + local insert="$2" + local infile="$3" + local outfile="$4" + + # sed append cmd: 'a\' + newline + text + newline + cmd="$(printf "a\\%b$insert%b" "\n" "\n")"
It seems I've managed the same with a simpler construct:
cmd="$(printf "a$insert\n")"
scripts/config checkarg() { · line 72
Quoted 1 line
scripts/config +1
-0+ sed -e "/$anchor/$cmd" "$infile" >"$outfile"
Why don't you handle the temp file in the helpers? (OK, I got it later, see below.)
[--SNIP--]
scripts/config checkarg() { · line 99
Quoted 9 lines
set_var(){scripts/config +2
-1local name=$1 new=$2 before=$3 name_re="^($name=|# $name is not set)" before_re="^($before=|# $before is not set)" if test -n "$before" && grep -Eq "$before_re" "$FN"; then - sed -ri "/$before_re/a $new" "$FN" + txt_append "^$before=" "$new" "$FN" "$FN.swp" + txt_append "^# $before is not set" "$new" "$FN.swp" "$FN"
What about doing it in a single run:
txt_append "^\($before=\|# $before is not set\)" [...]
Review
Quoted 1 line
(quoted diff) +1
-0+ rm "$FN.swp"
OK, this is sneaky. Since you use two calls, you use arg-swapping in the second call so that the temp file is the source and the actual file is the destination. That made me wonder why on Earth you would just 'rm' that tempfile without doing anything with it. Sneaky, I almost missed that. At least, put in a comment when using such tricks.
Besides, that won't work if you do it all in one pass, as shown above.
So:
- do the munging in one pass
- move handling the temp file in the helpers
scripts/config checkarg() { · line 104
Quoted 5 lines
scripts/config +3
-1elif grep -Eq "$name_re" "$FN"; then - sed -ri "s:$name_re.*:$new:" "$FN" + txt_subst "^$name=.*" "$new" "$FN" "$FN.swp" + txt_subst "^# $name is not set" "$new" "$FN.swp" "$FN" + rm "$FN.swp"
Ditto (one pass & temp file).
scripts/config set_var() { · line 115
Quoted 11 lines
scripts/config +3
-1else echo "$new" >>"$FN" fi@@ -77,7 +110,9 @@ set_var() { undef_var() { local name=$1 - sed -ri "/^($name=|# $name is not set)/d" "$FN" + txt_delete "^$name=" "$FN" "$FN.swp" + txt_delete "^# $name is not set" "$FN.swp" "$FN" + rm "$FN.swp"
Ditto (one pass & temp file).
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
Yann,
Thanks (once more!) for the careful review. See my answers inline.
2013/7/13 Yann E. MORIN <[email protected]>:
Quoted 11 lines
Clément, All,
On 2013-07-13 00:26 +0200, Clement Chauplannaz spake thusly:
Quoted 7 lines
Script
config' relies on extensions ofGNU sed', and is thus not working on all Unixes:
- in-place edition of files (-i), which can be replaced with a temporary file;
- extended-regexps (-r), which can be split into basic regexps;
- single-line calls to `a' command, while some implementations require a leading newline before the parameter.
Quoted 4 lines
Just nit-picking on the commit log: can you indent the bullet-list, and the continuation lines, so it is easier to read, like:
- in-place edition of files (-i), which can be replaced witha temporary file;
Ack. Applied.
Quoted 15 lines
Quoted 4 lines
Rewrite calls to `sed' to comply with POSIX interface, and move them to helper functions.
Signed-off-by Clement Chauplannaz <[email protected]>[--SNIP--]
scripts/config checkarg() { · line 70
Quoted 8 lines
scripts/config +8
-0+txt_append() { + local anchor="$1" + local insert="$2" + local infile="$3" + local outfile="$4" + + # sed append cmd: 'a\' + newline + text + newline + cmd="$(printf "a\\%b$insert%b" "\n" "\n")"It seems I've managed the same with a simpler construct:
Quoted 1 line
cmd="$(printf "a$insert\n")"
This does work with GNU-sed, but not with BSD-sed which does require separating the command character 'a' from its argument with a backslash (as per sed's POSIX definition).
Quoted 4 lines
Quoted 1 line
+ sed -e "/$anchor/$cmd" "$infile" >"$outfile"
Why don't you handle the temp file in the helpers? (OK, I got it later, see below.)
This makes it clearer indeed. Applied.
Quoted 12 lines
[--SNIP--]
scripts/config checkarg() { · line 99
Quoted 9 lines
set_var() { local name=$1 new=$2 before=$3 name_re="^($name=|# $name is not set)" before_re="^($before=|# $before is not set)" if test -n "$before" && grep -Eq "$before_re" "$FN"; thenscripts/config +2
-1- sed -ri "/$before_re/a $new" "$FN" + txt_append "^$before=" "$new" "$FN" "$FN.swp" + txt_append "^# $before is not set" "$new" "$FN.swp" "$FN"
Quoted 2 lines
What about doing it in a single run:
txt_append "^\($before=\|# $before is not set\)" [...]
This also works with GNU-sed, but not with BSD-sed. Alternation (matching either one of the strings separated by the vertical line) is not supported in Basic Regular Expressions; this is a GNU-sed extension.
Quoted 8 lines
Quoted 1 line
+ rm "$FN.swp"
OK, this is sneaky. Since you use two calls, you use arg-swapping in the second call so that the temp file is the source and the actual file is the destination. That made me wonder why on Earth you would just 'rm' that tempfile without doing anything with it. Sneaky, I almost missed that. At least, put in a comment when using such tricks.
Quoted 4 lines
Besides, that won't work if you do it all in one pass, as shown above.
So:
- do the munging in one pass
Cannot apply, for the reasons exposed above.
Quoted 1 line
- move handling the temp file in the helpers
Ack. Done.
Quoted 7 lines
scripts/config checkarg() { · line 104
Quoted 5 lines
elif grep -Eq "$name_re" "$FN"; then
scripts/config +3
-1- sed -ri "s:$name_re.*:$new:" "$FN" + txt_subst "^$name=.*" "$new" "$FN" "$FN.swp" + txt_subst "^# $name is not set" "$new" "$FN.swp" "$FN" + rm "$FN.swp"Ditto (one pass & temp file).
Ack.
Quoted 8 lines
Quoted 8 lines
else echo "$new" >>"$FN" fiscripts/config +0
-0@@ -77,7 +110,9 @@ set_var() { undef_var() { local name=$1
Quoted 6 lines
scripts/config set_var() { · line 115
Quoted 4 lines
scripts/config +3
-1- sed -ri "/^($name=|# $name is not set)/d" "$FN" + txt_delete "^$name=" "$FN" "$FN.swp" + txt_delete "^# $name is not set" "$FN.swp" "$FN" + rm "$FN.swp"Ditto (one pass & temp file).
Ack.
Quoted 3 lines
Regards,
Yann E. MORIN.
I will send a v3 with the changes listed above.
Thanks & regards,
Clement