Breaking changes for V1 #95
Merged
Conversation
This file contains 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
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #95 +/- ##
==========================================
+ Coverage 89.48% 94.40% +4.92%
==========================================
Files 14 15 +1
Lines 1683 1735 +52
==========================================
+ Hits 1506 1638 +132
+ Misses 177 97 -80
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
jakobnissen
force-pushed
the
v1
branch
5 times, most recently
from
6036343 to
a1649be
Compare
jakobnissen
force-pushed
the
v1
branch
16 times, most recently
from
3617ebf to
ca4aad7
Compare
jakobnissen
force-pushed
the
v1
branch
5 times, most recently
from
6883b0f to
ea1f2b3
Compare
Instead of doing `re.actions[:exit] = [:foo]`, do `onexit(re, :foo)`. It's cleaner, and avoids messing with internal fields of struct.
There is no reason to use `import` over `using`. On the contrary, it muddles the difference between exported and unexported names, and also allows extending foreign methods without warning.
Currently, Automa allows you to add a :final action to a regex like "a*", where it is not possible to determine when reading a byte whether it is the final byte matching the regex. Current behaviour is to execute the action for every byte that could concievably be the final byte whether or not it is. Disallow this behaviour by throwing an error in re2nfa for :final actions in looping regex.
Many potential users of Automa are not interested in parsing from IOs, but only buffers. For those users, the IO-parsing functionality of Automa is not needed, and so there is no need for dependency on TranscodingStreams.
NFAs with ambiguities often contain multiple ambiguities. Displaying the simplest ambiguity when erroring makes debugging easier - especially compared to when the shown ambiguity can never happen due to another ambiguity.
An oversight in the ambiguity check meant that actions placed on non-epsilon edges were accidentally not included in the paths for validation. MWE: `compile(onfinal!(re"a", :a) | onfinal!(re"a", :b))` This breaks tokenizers, so we manually skip ambiguity check in tokenizers. In the case of conflicting actions in tokenizers, this will cause the longest matching token to be emitted.
The tokenizer has a completely new design and API. * It's now much easier to use * It's now lazy by default * It's much faster, although not completely optimised. Its API is amenable to further optimisation * It handles errors automatically See issue BioJulia#116
Users should not have access to the module directly. Instead, export the RE struct, and also allow users to construct regex with `RE(str)`.
Instead of buffering an entire line, simply keep track of the number of columns cleared from the buffer. This reaches some more into TranscodingStreams privates, but it's well tested.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Superseded by #119
Todo
Closes #71
Closes #80
Closes #82
Closes #91
Closes #111
Closes #115
Closes #116