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(a)redhat.com&gt; > --- > 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).

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(a)redhat.com&gt; >>> --- >>> 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.

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(a)redhat.com&gt; > > > > > --- > > > > > 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.

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(a)redhat.com&gt; > > > > > > > --- > > > > > > > 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 :| -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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(a)redhat.com&gt; --- 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)