[libvirt] [PATCH] check-symfile: Use pythonesque string formatting instead of perl
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error: 'Symbol $1 is listed twice' Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";") if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr) ret = 1 else: wantsyms[line] = True -- 2.23.0
On 11/25/19 4:37 PM, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3). Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and started using string interpolation (or f-strings if you want). Reviewed-by: Erik Skultety <eskultet@redhat.com>
On 11/25/19 4:58 PM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3).
Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings are horrible. This is the most readable style for us, C developers IMO. Michal
On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
On 11/25/19 4:58 PM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3).
Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings are horrible. This is the most readable style for us, C developers IMO.
Can you be more specific on what exactly is horrible about f-strings? IMO it's actually very intuitive way of formatting strings unlike using the '%' formatting sign where depending on whether you have 1 or multiple arguments you may or may not need to use a tuple. F-strings are also a bit faster than the other formatting methods and because they're evaluated during runtime, you can evaluate arbitrary expressions, even call functions. Erik
On 11/26/19 9:24 AM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
On 11/25/19 4:58 PM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3).
Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings are horrible. This is the most readable style for us, C developers IMO.
Can you be more specific on what exactly is horrible about f-strings? IMO it's actually very intuitive way of formatting strings unlike using the '%' formatting sign where depending on whether you have 1 or multiple arguments you may or may not need to use a tuple. F-strings are also a bit faster than the other formatting methods and because they're evaluated during runtime, you can evaluate arbitrary expressions, even call functions.
That's exactly what I find horrible. Just consider the following example: print(f'a={f(x,n):d}, b={g(x,n):d}') IMO the following is more readable: print("a=%d, b=%d" % (f(x,n), g(x,n))) Once again, I'm talking about C developers (me specifically). I don't doubt that an experienced python developer finds f-strings a step forward. Michal
On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
On 11/26/19 9:24 AM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
On 11/25/19 4:58 PM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3).
Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings are horrible. This is the most readable style for us, C developers IMO.
Can you be more specific on what exactly is horrible about f-strings? IMO it's actually very intuitive way of formatting strings unlike using the '%' formatting sign where depending on whether you have 1 or multiple arguments you may or may not need to use a tuple. F-strings are also a bit faster than the other formatting methods and because they're evaluated during runtime, you can evaluate arbitrary expressions, even call functions.
That's exactly what I find horrible. Just consider the following example:
print(f'a={f(x,n):d}, b={g(x,n):d}')
IMO the following is more readable:
print("a=%d, b=%d" % (f(x,n), g(x,n)))
Once again, I'm talking about C developers (me specifically). I don't doubt that an experienced python developer finds f-strings a step forward.
I've plenty of python dev experiance and I still think f-strings are a horrible regression in design. IMHO it is a good thing to have the parameters visually separated from the string, as it makes it clearer to read the string being formatted. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Nov 26, 2019 at 10:01:11AM +0000, Daniel P. Berrangé wrote:
On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
On 11/26/19 9:24 AM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
On 11/25/19 4:58 PM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3).
Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings are horrible. This is the most readable style for us, C developers IMO.
Can you be more specific on what exactly is horrible about f-strings? IMO it's actually very intuitive way of formatting strings unlike using the '%' formatting sign where depending on whether you have 1 or multiple arguments you may or may not need to use a tuple. F-strings are also a bit faster than the other formatting methods and because they're evaluated during runtime, you can evaluate arbitrary expressions, even call functions.
That's exactly what I find horrible. Just consider the following example:
print(f'a={f(x,n):d}, b={g(x,n):d}')
IMO the following is more readable:
print("a=%d, b=%d" % (f(x,n), g(x,n)))
Once again, I'm talking about C developers (me specifically). I don't doubt that an experienced python developer finds f-strings a step forward.
I've plenty of python dev experiance and I still think f-strings are a horrible regression in design. IMHO it is a good thing to have the parameters visually separated from the string, as it makes it clearer to read the string being formatted.
I tend to agree here. f-strings are not that bad but it allows you to write ugly code. It is probably good for interactive python but otherwise I would prefer the new-style .format() with named arguments: "Hello {name}".format(name="Pavel") The major benefit that I see in this style is that it helps translators to figure out what the substitution will be and the order of arguments doesn't matter and you can build the dictionary passed to format() outside of it's call. Pavel
On Tue, Nov 26, 2019 at 11:25:28AM +0100, Pavel Hrdina wrote:
On Tue, Nov 26, 2019 at 10:01:11AM +0000, Daniel P. Berrangé wrote:
On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
On 11/26/19 9:24 AM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
On 11/25/19 4:58 PM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote: > Commit d30a1ad0443 translated the symbol file checker from perl to > python by doing a literal translation in most cases. Unfortunately one > string formatting operation was not really translated into python > leaving users with non-helpful error: > > 'Symbol $1 is listed twice' > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- > scripts/check-symfile.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py > index 0c02591991..34396b8623 100755 > --- a/scripts/check-symfile.py > +++ b/scripts/check-symfile.py > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: > line = line.strip(";") > > if line in wantsyms: > - print("Symbol $1 is listed twice", file=sys.stderr) > + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3).
Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings are horrible. This is the most readable style for us, C developers IMO.
Can you be more specific on what exactly is horrible about f-strings? IMO it's actually very intuitive way of formatting strings unlike using the '%' formatting sign where depending on whether you have 1 or multiple arguments you may or may not need to use a tuple. F-strings are also a bit faster than the other formatting methods and because they're evaluated during runtime, you can evaluate arbitrary expressions, even call functions.
That's exactly what I find horrible. Just consider the following example:
print(f'a={f(x,n):d}, b={g(x,n):d}')
IMO the following is more readable:
print("a=%d, b=%d" % (f(x,n), g(x,n)))
Once again, I'm talking about C developers (me specifically). I don't doubt that an experienced python developer finds f-strings a step forward.
I've plenty of python dev experiance and I still think f-strings are a horrible regression in design. IMHO it is a good thing to have the parameters visually separated from the string, as it makes it clearer to read the string being formatted.
I tend to agree here. f-strings are not that bad but it allows you to write ugly code. It is probably good for interactive python but otherwise I would prefer the new-style .format() with named arguments:
"Hello {name}".format(name="Pavel")
The major benefit that I see in this style is that it helps translators to figure out what the substitution will be and the order of arguments doesn't matter and you can build the dictionary passed to format() outside of it's call.
Note those advantages are already possible with traditional syntax "Hello %(name)s" % { name: "Pavel"} but for our build scripts this is academic as we're never going to do translation of anything in them. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Nov 26, 2019 at 10:29:14AM +0000, Daniel P. Berrangé wrote:
On Tue, Nov 26, 2019 at 11:25:28AM +0100, Pavel Hrdina wrote:
On Tue, Nov 26, 2019 at 10:01:11AM +0000, Daniel P. Berrangé wrote:
On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
On 11/26/19 9:24 AM, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
On 11/25/19 4:58 PM, Erik Skultety wrote: > On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote: > > Commit d30a1ad0443 translated the symbol file checker from perl to > > python by doing a literal translation in most cases. Unfortunately one > > string formatting operation was not really translated into python > > leaving users with non-helpful error: > > > > 'Symbol $1 is listed twice' > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > --- > > scripts/check-symfile.py | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py > > index 0c02591991..34396b8623 100755 > > --- a/scripts/check-symfile.py > > +++ b/scripts/check-symfile.py > > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: > > line = line.strip(";") > > > > if line in wantsyms: > > - print("Symbol $1 is listed twice", file=sys.stderr) > > + print("Symbol %s is listed twice" % line ,file=sys.stderr) > > Not a deal breaker, but IMO should at least the "new" syntax for string > formatting using the .format() method (works both with python 2 and 3). > > Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and > started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings are horrible. This is the most readable style for us, C developers IMO.
Can you be more specific on what exactly is horrible about f-strings? IMO it's actually very intuitive way of formatting strings unlike using the '%' formatting sign where depending on whether you have 1 or multiple arguments you may or may not need to use a tuple. F-strings are also a bit faster than the other formatting methods and because they're evaluated during runtime, you can evaluate arbitrary expressions, even call functions.
That's exactly what I find horrible. Just consider the following example:
print(f'a={f(x,n):d}, b={g(x,n):d}')
IMO the following is more readable:
print("a=%d, b=%d" % (f(x,n), g(x,n)))
Once again, I'm talking about C developers (me specifically). I don't doubt that an experienced python developer finds f-strings a step forward.
I've plenty of python dev experiance and I still think f-strings are a horrible regression in design. IMHO it is a good thing to have the parameters visually separated from the string, as it makes it clearer to read the string being formatted.
I tend to agree here. f-strings are not that bad but it allows you to write ugly code. It is probably good for interactive python but otherwise I would prefer the new-style .format() with named arguments:
"Hello {name}".format(name="Pavel")
The major benefit that I see in this style is that it helps translators to figure out what the substitution will be and the order of arguments doesn't matter and you can build the dictionary passed to format() outside of it's call.
Note those advantages are already possible with traditional syntax
"Hello %(name)s" % { name: "Pavel"}
but for our build scripts this is academic as we're never going to do translation of anything in them.
The advantage of "".format() is that you cannot make one mistake that can happen with the currently used version and that is that sometimes you might need to make sure the variable is in a tuple, i.e. difference between `params` and `(params,)`. Even though this is *very* corner case, you still don't know if the word after `%` is the only parameter or the iterable of parameters to be used. Even though this is not *that* corner case, the mistakes that would cause this would probably be caught quickly and auxiliary build scripts are not something that needs to care about translations. Since the biggest bikeshedding^Wconversations usually revolve around the smallest patches, I felt like expressing myself as well =) But anyway, even though "%d" is more C-like, when talking about the parameters, the `.format()` is more C-like IMHO. More importantly it should be the same in all of the code, so that it is consistent. And *most* importantly!!! It is just a auxiliary build script, just make sure it works and that's it =D Have a nice day (with my preferred colour of the bikeshed), Martin P.S.: Just to illustrate the first mentioned difference, try reading the following code. I think the `.format` makes it more understandable: def a(*args): print("{}".format(args)) def b(*args): print("{}".format(*args)) def c(*args): print("%r" % (args,)) def d(*args): print("%r" % (*args,)) def e(*args): print("%r" % args) a('asdf') b('asdf') c('asdf') d('asdf') e('asdf') a('asdf', 'fdsa') b('asdf', 'fdsa') c('asdf', 'fdsa') #d('asdf', 'fdsa') # it easy to see why this fails, but the code is gross #e('asdf', 'fdsa') # it is not easy to see why this fails a() #b() # it is easy to see why this fails c() #d() # it easy to see why this fails, but the code is gross #e() # it is not easy to see why this fails
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Nov 28, 2019 at 11:43:01AM +0100, Martin Kletzander wrote:
The advantage of "".format() is that you cannot make one mistake that can happen with the currently used version and that is that sometimes you might need to make sure the variable is in a tuple, i.e. difference between `params` and `(params,)`. Even though this is *very* corner case, you still don't know if the word after `%` is the only parameter or the iterable of parameters to be used. Even though this is not *that* corner case, the mistakes that would cause this would probably be caught quickly and auxiliary build scripts are not something that needs to care about translations.
Since the biggest bikeshedding^Wconversations usually revolve around the smallest patches, I felt like expressing myself as well =)
But anyway, even though "%d" is more C-like, when talking about the parameters, the `.format()` is more C-like IMHO. More importantly it should be the same in all of the code, so that it is consistent. And *most* importantly!!! It is just a auxiliary build script, just make sure it works and that's it =D
*an auxiliary
Have a nice day (with my preferred colour of the bikeshed), Martin
Best wishes, Jano
Michal Privoznik <mprivozn@redhat.com> [2019-11-26, 09:48AM +0100]:
That's exactly what I find horrible. Just consider the following example:
print(f'a={f(x,n):d}, b={g(x,n):d}')
IMO the following is more readable:
print("a=%d, b=%d" % (f(x,n), g(x,n)))
First of all, the format specifiers (i.e, ":d") are completely optional if you just care about the string representation of an object. Secondly, I certainly hope that such code never passes code review anywhere. Properly cleaning this up to a = f(x,n) b = g(x,n) print(f"a={a}, b={b}") is much more readable then the old-style print("a=%d, b=%d" % (a, b)) or even the .format-style print("a={}, b={}".format(a, b)) Also, format strings have the best performance for string interpolation in python which is a non-bikeshed argument for them. Bjoern -- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon, Nov 25, 2019 at 16:58:39 +0100, Erik Skultety wrote:
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string formatting using the .format() method (works both with python 2 and 3).
This rest of this script uses the % syntax so I'm not going to add a different style into this file nor do a conversion of irrelevant parts in this patch. Obviously if you have suggestions you can update the coding style guidelines and/or convert the scripts to use the more pythonic syntax.
On Mon, Nov 25, 2019 at 4:41 PM Peter Krempa <pkrempa@redhat.com> wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Nitpick, 'line ,file=...' -> 'line, file=...'
On Tue, Nov 26, 2019 at 09:51:18AM +0100, Fabiano Fidêncio wrote:
On Mon, Nov 25, 2019 at 4:41 PM Peter Krempa <pkrempa@redhat.com> wrote:
Commit d30a1ad0443 translated the symbol file checker from perl to python by doing a literal translation in most cases. Unfortunately one string formatting operation was not really translated into python leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0c02591991..34396b8623 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -52,7 +52,7 @@ with open(symfile, "r") as fh: line = line.strip(";")
if line in wantsyms: - print("Symbol $1 is listed twice", file=sys.stderr) + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Nitpick, 'line ,file=...' -> 'line, file=...'
These are easily caught by syntax-check: ../scripts/check-symfile.py:55:53: E203 whitespace before ',' print("Symbol %s is listed twice" % line ,file=sys.stderr) ^ ../scripts/check-symfile.py:55:54: E231 missing whitespace after ',' print("Symbol %s is listed twice" % line ,file=sys.stderr) ^ make: *** [../build-aux/syntax-check.mk:917: sc_flake8] Error 123 make: *** Waiting for unfinished jobs.... Jano
participants (9)
-
Bjoern Walk -
Daniel P. Berrangé -
Erik Skultety -
Fabiano Fidêncio -
Ján Tomko -
Martin Kletzander -
Michal Privoznik -
Pavel Hrdina -
Peter Krempa