4:43 a.m.
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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list