WString: unify numeric conversion and fix assignments by mcspr · Pull Request #8526 · esp8266/Arduino (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation3 Commits9 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Restore the pre-3.0.0 behaviour when we could assign numeric values to
the string object. After introducing operator =(char), everything was
converted to char instead of the expected 'stringification' of the
number (built-in int, long, unsigned int, unsigned long, long long,
unsigned long long, float and double)
Add toString() that handles conversions, re-use it through out the class
Fix #8430
Restore the pre-3.0.0 behaviour when we could assign numeric values to
the string object. After introducing operator =(char), everything was
converted to char instead of the expected 'stringification' of the
number (built-in int, long, unsigned int, unsigned long, long long,
unsigned long long, float and double)
I suppose there should be tests for assignment... Since there are for constructors.
Tests added
… decimalPlaces
branch via if (base == 10) { sprintf(...) } else { ... } instead of separate funcs reuse the constructor for numeric types where it's possible
nonstd arduino funcs expect this usage pattern
Comment on lines -67 to -77
| explicit String(unsigned char, unsigned char base = 10); |
|---|
| explicit String(int, unsigned char base = 10); |
| explicit String(unsigned int, unsigned char base = 10); |
| explicit String(long, unsigned char base = 10); |
| explicit String(unsigned long, unsigned char base = 10); |
| explicit String(long long /* base 10 */); |
| explicit String(long long, unsigned char base); |
| explicit String(unsigned long long /* base 10 */); |
| explicit String(unsigned long long, unsigned char base); |
| explicit String(float, unsigned char decimalPlaces = 2); |
| explicit String(double, unsigned char decimalPlaces = 2); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is mostly for verbosity. Due to the amount of magic ctors we have... we can't really have std::initializer_list<char> ctor as well, since everything would be converted into it
e.g.
String(std::initializer_list data) { concat(data.begin(), data.size()); }
So only purpose of the below is to allow to omit String{...} in front of the arguments list. Where we suppose to have explicit for String(int, base) overload
core/test_string.cpp:620:51: error: converting to ‘String’ from initializer list would use explicit constructor ‘String::String(int, unsigned char)’ 620 | String strings[] {{62, 10}, {63, 10}, {64, 10}}; | ^
Could be reverted though.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are nice and seem to cover most of the cases, if not all.
Thanks to restore the API and factorize code
that may beclosed by this pull request
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request
Restore the pre-3.0.0 behaviour when we could assign numeric values to the string object. After introducing operator =(char), everything was converted to char instead of the expected 'stringification' of the number (built-in int, long, unsigned int, unsigned long, long long, unsigned long long, float and double)
Add toString() that handles conversions, re-use it through out the class
Fix esp8266#8430
2 participants
