Add "infinity", "negativeInfinity", and "nan" keywords to rfwtxt. #5899
Conversation
| if (identifier == 'false') { | ||
| _advance(); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Should these identifiers be added to the reserved words list?
There was a problem hiding this comment.
This is a blocking comment. These identifiers need to be added to the reserved words list; otherwise they could get shadowed by builders (when landed) by doing something like
Builder(
builder: (infinity) => Container(width: infinity.width),
),
There was a problem hiding this comment.
Hmm. If we add them to the reserved words list then this becomes a breaking change.
There was a problem hiding this comment.
Wondering if something like flutter/flutter#141833 (comment) would be useful here as well (instead of changing the language). The implementation of number could be as follows.
final result = _fetch(...);
if (result is int) return result as int;
if (result is double) return result as double;
if (result == 'infinity') return double.infinity;
if (result == 'nan') return double.nan;
There was a problem hiding this comment.
That would similarly be a breaking change though.
| repository: https://github.com/flutter/packages/tree/main/packages/rfw | ||
| issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+rfw%22 | ||
| version: 1.0.16 | ||
| version: 1.0.19 |
There was a problem hiding this comment.
Is this expected? Why are 2 versions being skipped?
There was a problem hiding this comment.
i have a bunch of other PRs for this package and gave them all different numbers. I'll pick the right one when I land it. :-)
| expect(parseDataFile('{ "a": -1e-19 }'), <String, Object?>{ 'a': -0.0000000000000000001 }); | ||
| expect(parseDataFile('{ "a": infinity }'), <String, Object?>{ 'a': double.infinity }); | ||
| expect(parseDataFile('{ "a": negativeInfinity }'), <String, Object?>{ 'a': double.negativeInfinity }); | ||
| expect(parseDataFile('{ "a": nan }')['a'], isNaN); |
There was a problem hiding this comment.
nit: For the sake of consistency, should this particular test be
expect(..., <String, Object?> { 'a': double.nan });
There was a problem hiding this comment.
that would fail, because double.nan == double.nan is false
peixinli
left a comment
peixinli
left a comment
There was a problem hiding this comment.
Thanks for adding this!
|
We could make infinity and negative infinity be written as |
|
We could also use |
Hixie
dismissed
peixinli’s stale review
canceling lgtm to make sure i don't accidentally land this with the breaking change
|
From triage: what's the status of this PR? Should it be a Draft? |
|
There doesn't seem to be a huge amount of interest here so I'm tempted to just close it for now until we have a clearer need. |
|
closing per last comment. If you would like this feature please file a bug and cc me. Thanks! |
4 participants
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).