-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Feature: str.format accepting !r, !s and !a #1027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
By the Python docs, the `!` forces a conversion of the argument before applying the normal formating logic described after the `:` marker. See: https://docs.python.org/3.4/library/string.html#format-string-syntax
@palaviv: Can you please take a look if it suits your needs? |
Codecov Report
@@ Coverage Diff @@
## master #1027 +/- ##
==========================================
+ Coverage 64.71% 64.74% +0.03%
==========================================
Files 97 97
Lines 17047 17093 +46
Branches 3800 3811 +11
==========================================
+ Hits 11032 11067 +35
- Misses 3441 3443 +2
- Partials 2574 2583 +9
Continue to review full report at Codecov.
|
|
||
match chars.next() { | ||
None => None, // Should fail instead? | ||
Some(c) => FormatPreconversor::from_char(c), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can just be chars.next().map(FormatPreconversor::from_char)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum... this still returns None if chars.next()
is None?
Asking because FormatPreconversor::from_char
is not prepared to handle None as input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another aspect: // Should fail instead?
I do not know how to raise exceptions. Ideally, {!x}
should fail, but this code will pass for now:
# CPy 3.6
>>> '{!x}'.format(123)
Traceback (most recent call last):
File "<ipython-input-2-b294f18f6498>", line 1, in <module>
'{!x}'.format(123)
ValueError: Unknown conversion specifier x
# RustPython
>>>>> '{!x}'.format(123)
'123'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
next().map(FormatPreconversor::from_char)
would just map the inner value of the Option
if present – would do exactly what you did (pass the inner value to from_char()
if it exists, or return None if the Option
is None
).
To raise an exception, you’d need to signal an error somehow (eg. return a Result<Option<FormatPreconversor>, String>
instead of an Option<FormatPreconversor>
) and then check for the result in call_object_format()
and in case of an error, return a new vm error there (eg. by return Err(vm.new_value_error(…))
). Or just by mapping the error:
let (preconversor, new_format_spec) = FormatPreconversor::parse_and_consume(format_spec);
let preconversor = preconversor.map_err(|msg| vm.new_value_error(msg))?;
// …
But then you’d probably want to return Result<(Option<FormatPreconversor>, &str), String>
rather than (Result<Option<FormatPreconversor>, String>, &str)
, so that it’d look like:
let (preconversor, new_format_spec) = FormatPreconversor::parse_and_consume(format_spec)
.map_err(|msg| vm.new_value_error(msg))?;
From: https://gitter.im/rustpython/Lobby?at=5cfa898ebf4cbd167c484cf0
As the Python docs, when a
{!r}
occur, it runs__repr__
over the argument before applying the:
rules. The current implementation of{!a}
is not correct, as simple acts as!r
.