Add some extra functionality. #34
Conversation
Also do some clean-up. Fixes #21, #22 and #24: * https://github.com/dart-lang/characters/issues/24 * https://github.com/dart-lang/characters/issues/22 * https://github.com/dart-lang/characters/issues/21
CHANGELOG.md
Outdated
| @@ -1,8 +1,15 @@ | |||
| ## 1.2.0-nullsafety | |||
There was a problem hiding this comment.
There hasn't been a stable 1.1.0 release yet - I would just do 1.1.0-nullsafety.3
lib/src/characters_impl.dart
Outdated
| Characters characterAt(int position) { | ||
| var breaks = Breaks(string, 0, string.length, stateSoTNoBreak); | ||
| var start = 0; | ||
| findIndices: |
There was a problem hiding this comment.
I am personally not a fan of using labels in dart - they are just really uncommon and thus can be confusing or require additional research to understand exactly what they do (I had no idea you could even use them to break out from a whole block like this).
Consider just inlining the two throw statements instead of doing break findIndices; which really just throws - or call a method that throws. That has a lot more straightforward/explicit behavior imo.
There was a problem hiding this comment.
With that attitude, there won't ever be common use of labels 😉 .
This code is probably just as good with inlined throws. Will do.
FYI: You can label any statement, and you can break any surrounding label. It only really makes sense for composite statements (but you can write foo: break foo;). In that way, Dart precisely copied Java and JavaScript, so it's not anything fancy. Since Dart does not have loop else branches, like Python, I sometimes write what should be for (...) { ... if (...) break; ... } else { ... else code ... } as:
outer: {
for (...) {
...
if (...) break outer;
...
}
... else code ...
}
lib/src/characters_impl.dart
Outdated
| return StringCharacters(string.substring(0, endIndex)); | ||
| var end = breaks.nextBreak(); | ||
| if (end < 0) break findIndices; | ||
| ; |
There was a problem hiding this comment.
That ; looks unnecessary :)
lib/src/characters_impl.dart
Outdated
| Characters characterAt(int position) { | ||
| var breaks = Breaks(string, 0, string.length, stateSoTNoBreak); | ||
| var start = 0; | ||
| findIndices: |
There was a problem hiding this comment.
With that attitude, there won't ever be common use of labels 😉 .
This code is probably just as good with inlined throws. Will do.
FYI: You can label any statement, and you can break any surrounding label. It only really makes sense for composite statements (but you can write foo: break foo;). In that way, Dart precisely copied Java and JavaScript, so it's not anything fancy. Since Dart does not have loop else branches, like Python, I sometimes write what should be for (...) { ... if (...) break; ... } else { ... else code ... } as:
outer: {
for (...) {
...
if (...) break outer;
...
}
... else code ...
}Travis won't run tests with the current set-up, the source span gets a version which doesn't work. Try a later SDK version to see if that helps.
pubspec.yaml
Outdated
| @@ -5,7 +5,7 @@ homepage: https://www.github.com/dart-lang/characters | |||
|
|
|||
| environment: | |||
| # This must remain a tight constraint until nnbd is stable | |||
| sdk: '>=2.10.0-78 <2.10.0' | |||
| sdk: '>=2.10.0-137 <2.10.0' | |||
There was a problem hiding this comment.
Note that this might be past the 2.10 stable branch point (and never included in 2.10)
There was a problem hiding this comment.
See dart-lang/sdk#43485, also please mention there that you also were mislead by this
There was a problem hiding this comment.
I wasn't actually misled, I was just guessing wildly at why Travis gives errors in the sourcespan package, so I updated to the same SDK version as that package. Didn't work, will revert.
Add some extra functionality Also do some clean-up. Fixes dart-lang/characters#21, dart-lang/characters#22 and dart-lang/characters#24: * https://github.com/dart-lang/characters/issues/24 * https://github.com/dart-lang/characters/issues/22 * https://github.com/dart-lang/characters/issues/21
Also do some clean-up.
Fixes dart-lang/core#378, dart-lang/core#379 and dart-lang/core#381: