Skip to content
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

Conversation

mlorbetske
Copy link
Contributor

Do not merge - these are only here for visibility for now

This PR contains two things:

  1. The new Trie that addresses Use the new Trie for processing and adjust buffer mechanics accordingly #6 - either the Trie will need to be changed or the IOperation interface will need to absorb the requirement for tracking the absolute position in content streams
  2. The new command parser to break the dependency on CLI.Utils (there's also a dependency for console colors & invoking other dotnet commands that will need to be dealt with separately)

@seancpeters

@mlorbetske mlorbetske force-pushed the dev/mlorbe/AddSpikesForCommandParserAndNewTrie branch from 5c7ad9e to c367ed0 Compare March 10, 2017 06:51
@mlorbetske
Copy link
Contributor Author

mlorbetske commented Mar 10, 2017

Remaining work:

  • Allow the specification of lookarounds in configuration
  • Add tests that assert that TokenTrie plays nice with lookarounds

@mlorbetske mlorbetske force-pushed the dev/mlorbe/AddSpikesForCommandParserAndNewTrie branch from bfcd99a to ecb036a Compare March 11, 2017 18:39
@mlorbetske mlorbetske changed the title Add spikes for command parser and new trie Use the new unified Trie and add support for look-arounds Mar 11, 2017
@mlorbetske
Copy link
Contributor Author

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];
Copy link
Contributor

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.

Copy link
Contributor Author

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;)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while loop ?

Copy link
Contributor Author

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
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constructor possibly?

Copy link
Contributor Author

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)
{
Copy link
Contributor

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.

Copy link
Contributor

@seancpeters seancpeters left a 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!

@mlorbetske mlorbetske merged commit 85759b1 into dotnet:rel/vs2017/post-rtw Mar 15, 2017
@mlorbetske mlorbetske deleted the dev/mlorbe/AddSpikesForCommandParserAndNewTrie branch March 17, 2017 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants