-
Notifications
You must be signed in to change notification settings - Fork 376
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use the new unified Trie and add support for look-arounds #393
Use the new unified Trie and add support for look-arounds #393
Conversation
5c7ad9e
to
c367ed0
Compare
Remaining work:
|
bfcd99a
to
ecb036a
Compare
We'll make a new issue for adding full support for lookarounds for each operation (in its matched tokens). This PR is now ready. |
{ | ||
byte[] tmp = new byte[_trie.MaxLength]; | ||
byte[] tmp = new byte[_trie.MaxLength + 1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sizedToStrream check was removed with this PR. Wondering if there could be problems if the entire source stream is shorter than the _trie.MaxLength + 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is OK as the current buffer size is tracked differently
|
||
if (CurrentBufferLength == CurrentBuffer.Length && CurrentBufferLength - CurrentBufferPosition < _trie.MaxLength) | ||
//Loop until we run out of data in the buffer | ||
for(; CurrentBufferPosition < CurrentBufferLength;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while loop ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
namespace Microsoft.TemplateEngine.Core.Matching | ||
{ | ||
public abstract class TerminalBase | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constructor possibly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
public bool GetOperation(byte[] buffer, int bufferLength, ref int currentBufferPosition, out int token) | ||
public void AddToken(IToken token, int index) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any checking that multiple tokens don't have the same index. This would only potentially be a problem if tokens are added with explicit indexes.
OTOH, I don't have this fully internalized yet, so perhaps duplicated indexes are ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, looking forward to leveraging it!
…port seeking when doing sizing to streams, convert for with no initial condition or iteration action to a while loop
…s://github.com/mlorbetske/templating into dev/mlorbe/AddSpikesForCommandParserAndNewTrie
Do not merge - these are only here for visibility for now
This PR contains two things:
dotnet
commands that will need to be dealt with separately)@seancpeters