10000 cformat star asterisk should behave differently when given negative sign · Issue #4762 · RustPython/RustPython · GitHub
[go: up one dir, main page]
Skip to content

cformat star asterisk should behave differently when given negative sign #4762

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.

Sign up for GitHub

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

Jump to bottom
Open
MegasKomnenos opened this issue Mar 25, 2023 · 2 comments
Open

cformat star asterisk should behave differently when given negative sign #4762

MegasKomnenos opened this issue Mar 25, 2023 · 2 comments
Labels
A-stdlib C-bug Something isn't working z-ca-2023 Tag to track contrubution-academy 2023

Comments

@MegasKomnenos
Copy link
Contributor

In string formatting, a print-f style format specifier can have its amount and precision formatted by variable value through use of star asterik(%*s).

In cformat_bytes and cformat_string, through try_update_quantity_from_tuple, try_update_quantity_from_element is used to update quantity if possible, but it makes the assumption that its behavior should be the same regardless of the sign value of the given element.

if let Some(i) = width_obj.payload::<PyInt>() {
    let i = i.try_to_primitive::<i32>(vm)?.unsigned_abs();
    Ok(CFormatQuantity::Amount(i as usize))
}

However, that assumption is false, as an amount can be preceded by a negative sign, and it should make the resulting string left adjusted, according to format specifier rule.
https://docs.python.org/3/library/stdtypes.html#old-string-formatting

assert('%*s' % (-5, 'abc') == 'abc  ')

This issue makes this test to fail.

I think this issue can be resolved by checking the sign of the element and adding the minus sign accordingly.

I'm working on the fix at the moment.
MegasKomnenos added a commit to MegasKomnenos/RustPython that referenced this issue Mar 25, 2023
@MegasKomnenos
Fix issue RustPython#4762
4326547
MegasKomnenos added a commit to MegasKomnenos/RustPython that referenced this issue Mar 25, 2023
@MegasKomnenos
Fix RustPython#4762
84d91ab
@MegasKomnenos MegasKomnenos mentioned this issue Mar 25, 2023
Merged
@DimitrisJim
Copy link
Member

hah, first time I've heard of the asterisk tbh.

@MegasKomnenos
Copy link
Contributor Author

hah, first time I've heard of the asterisk tbh.

Same here lol. I wouldn't have known it if there wasn't a failing test that used it, haha.

@youknowone youknowone added C-bug Something isn't working A-stdlib z-ca-2023 Tag to track contrubution-academy 2023 labels Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-stdlib C-bug Something isn't working z-ca-2023 Tag to track contrubution-academy 2023
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@youknowone @DimitrisJim @MegasKomnenos
0