| Message ID | 1416227466-29491-3-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/17/2014 05:31 AM, Max Reitz wrote: > _filter_qmp should be able to correctly filter out the QMP version > object for pretty JSON output. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/common.filter | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter > index 3acdb30..dfb9726 100644 > --- a/tests/qemu-iotests/common.filter > +++ b/tests/qemu-iotests/common.filter > @@ -167,7 +167,9 @@ _filter_qmp() > { > _filter_win32 | \ > sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \ > - -e 's#^{"QMP":.*}$#QMP_VERSION#' > + -e 's#^{"QMP":.*}$#QMP_VERSION#' \ > + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ \s is a GNU sed extension. But we don't really need to care about whitespace to the end of the line; I think that it is sufficient to just match the following regex: -e '/^ "QMP": {/, /^ }/ c\' \ > + -e ' QMP_VERSION' Either way, it's not the first time we've used a GNU sed extension, and since other series are now depending on this one, I can live with: Reviewed-by: Eric Blake <eblake@redhat.com>
On 2014-11-17 at 17:04, Eric Blake wrote: > On 11/17/2014 05:31 AM, Max Reitz wrote: >> _filter_qmp should be able to correctly filter out the QMP version >> object for pretty JSON output. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/common.filter | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter >> index 3acdb30..dfb9726 100644 >> --- a/tests/qemu-iotests/common.filter >> +++ b/tests/qemu-iotests/common.filter >> @@ -167,7 +167,9 @@ _filter_qmp() >> { >> _filter_win32 | \ >> sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \ >> - -e 's#^{"QMP":.*}$#QMP_VERSION#' >> + -e 's#^{"QMP":.*}$#QMP_VERSION#' \ >> + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ > \s is a GNU sed extension. But we don't really need to care about > whitespace to the end of the line; I think that it is sufficient to just > match the following regex: > > -e '/^ "QMP": {/, /^ }/ c\' \ That doesn't match, however. QEMU's JSON formatter sometimes outputs whitespace at the end of line. >> + -e ' QMP_VERSION' > Either way, it's not the first time we've used a GNU sed extension, and > since other series are now depending on this one, I can live with: Ooooh, you mean version 3 was actually fine? (which used 'c\' without a line break) ;-) > Reviewed-by: Eric Blake <eblake@redhat.com> Thank you! Max
On 11/17/2014 09:14 AM, Max Reitz wrote: >>> + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ >> \s is a GNU sed extension. But we don't really need to care about >> whitespace to the end of the line; I think that it is sufficient to just >> match the following regex: >> >> -e '/^ "QMP": {/, /^ }/ c\' \ > > That doesn't match, however. QEMU's JSON formatter sometimes outputs > whitespace at the end of line. No, I explicitly omitted the $. The lines match because they are anchored to the beginning, regardless of trailing whitespace at the end. > >>> + -e ' QMP_VERSION' >> Either way, it's not the first time we've used a GNU sed extension, and >> since other series are now depending on this one, I can live with: > > Ooooh, you mean version 3 was actually fine? (which used 'c\' without a > line break) ;-) I'm more comfortable with v4 than v3 (\s is well-known, and can be easily converted into an alternative if someone proves their system doesn't support it. But c\ is not well-known; and reading 'info sed' to learn about c\ only documents its use with a newline; so using c\ without a newline is relying on undocumented GNU behavior, which is very risky). Of course, a v5 that avoids \s would avoid any confusion, but now we're bikeshedding, and I'm not sure it's worth the time.
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 3acdb30..dfb9726 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -167,7 +167,9 @@ _filter_qmp() { _filter_win32 | \ sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \ - -e 's#^{"QMP":.*}$#QMP_VERSION#' + -e 's#^{"QMP":.*}$#QMP_VERSION#' \ + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ + -e ' QMP_VERSION' } # replace driver-specific options in the "Formatting..." line
_filter_qmp should be able to correctly filter out the QMP version object for pretty JSON output. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/common.filter | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)