`disect` dependency not working as expected

[joshjung]

Hey there, after about 4-5 hours of fighting an issue I discovered that the disect library is not working as one would expect for use with the tokenizer.

Here is my use case:

t.addRule(/^([a-zA-Z0-9\-_]+\s*=\s*)(["'])(\\\2|[^"']+)*?\2$/, 'tKeyValue'); // Matches name='value'
t.addRule(/^[a-zA-Z0-9\-_]+$/, 'tIdentifier'); // Matches name

The line I am attempting to tokenize is (slim template code):

tag#id var1 = 'value1' var2 = 'value2'

For some reason, the disect library starts with the full length string and then halves the index value every single time it attempts another match, so it ends up trying to match the following strings:

tag#id var1 = 'value1' var2 = 'value2'
tag#id var1 = 'valu
tag#id

This does not have the desired result. I've fixed the issue in my fork and will be attempting a merge. However, I am not sure what testing suite is being used and would like to write some tests myself. What should I write them in?

[Floby]

Hello,
It seems to me you are trying to tokenize too big tokens. What is your expected output ?

[joshjung]

Hey there Floby! I've got a merge request going on right now. I've actually improved performance by an insane amount and included a unit test in my fork that includes the exact scenario I was not able to get working with the existing code - and all tests pass.

P.S. My input was only about 30 characters long and the token it was unable to match properly was only about 10 characters.

[Floby]

Hey,
I'll try and have a look at your pull request soon.
However the description of what the disect module does is pretty much what bisection is to me. But I would rather that it is not the best method for handling this case.

[joshjung]

Yeah @Floby , good point. Bisection yes should work that way but I just sortof assumed that since the tokenizer was working at all before it must have been doing a char-by-char comparison. It confused me as to why it was being used.

I'm going to add some notes to my merge request now. Let's hold off on merging it until my use case is resolved.

[joshjung]

So, I'm including a "minimum" case here explaining this issue:

var Tokenizer = require('tokenizer');

var t = new Tokenizer();

t.addRule(/^hello$/, 'hello');
t.addRule(/^world$/, 'world');
t.addRule(/^[AB]$/, 'A or B');
t.addRule(Tokenizer.whitespace);

t.on('token', function (token) {
  console.log(token);
});

t.write('A hello world B');

Output is:

{ content: 'A', type: 'A or B', toString: [Function] }
{ content: ' ', type: 'whitespace', toString: [Function] }

events.js:72
        throw er; // Unhandled 'error' event
              ^
SyntaxError: could not tokenize "hello world B"
    at Tokenizer._tokenize (/Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:68:13)
    at Tokenizer._tokenize (/Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:83:12)
    at Tokenizer._tokenize (/Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:83:12)
    at /Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:32:14
    at process._tickCallback (node.js:419:13)
    at Function.Module.runMain (module.js:499:11)
    at startup (node.js:119:16)
    at node.js:906:3

[Floby]

that's definitely a bug. Thank you

On 3 December 2014 at 16:41, Joshua Jung notifications@github.com wrote:

So, I'm including a "minimum" case here explaining this issue:

var Tokenizer = require('tokenizer');

var t = new Tokenizer();

t.addRule(/^hello$/, 'hello');
t.addRule(/^world$/, 'world');
t.addRule(/^[AB]$/, 'A or B');
t.addRule(Tokenizer.whitespace);

t.on('token', function (token) {
console.log(token);
});

t.write('A hello world B');

Output is:

{ content: 'A', type: 'A or B', toString: [Function] }
{ content: ' ', type: 'whitespace', toString: [Function] }

events.js:72
throw er; // Unhandled 'error' event
^
SyntaxError: could not tokenize "hello world B"
at Tokenizer._tokenize (/Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:68:13)
at Tokenizer._tokenize (/Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:83:12)
at Tokenizer._tokenize (/Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:83:12)
at /Users/jjung/dev/tokenizer-test/node_modules/tokenizer/lib/Tokenizer.js:32:14
at process._tickCallback (node.js:419:13)
at Function.Module.runMain (module.js:499:11)
at startup (node.js:119:16)
at node.js:906:3

—
Reply to this email directly or view it on GitHub
#8 (comment).

[joshjung]

So to provide more information, I decided to do some debugging. I updated tokenizer.js as follows:

...
var maxIndex = disect(0, data.length, function (index) {
  var buf = data.substring(0, index + 1);
  console.log(buf); // Adding debugging output here
  return self._getMatchingRule(buf) === null;
});
...

Output of running the former code I posted in the previous message is:

A hello
A he
A
A
{ content: 'A', type: 'A or B', toString: [Function] }
 hello w
 hel
 h

{ content: ' ', type: 'whitespace', toString: [Function] }
hello w
hell
he
h

events.js:72
        throw er; // Unhandled 'error' event
              ^
SyntaxError: could not tokenize "hello world B"

The problem is that by design the tokenizer is not matching anything unless the string starts with h and ends with o. But because disect skips from hello w to hell it can never match.

The fork that I'm working on completely removes disect because I do not see how it is even possible to require a perfect match on tokens and yet to skip some characters.

Thoughts?

[Floby]

My thoughts are you're right. If you can add this case in a unit test of
the suite I'll be glad to merge your fork

On 3 December 2014 at 17:00, Joshua Jung notifications@github.com wrote:

So to provide more information, I decided to do some debugging. I updated
tokenizer.js as follows:

var maxIndex = disect(0, data.length, function (index) {
var buf = data.substring(0, index + 1);
console.log(buf); // Adding debugging output here
return self._getMatchingRule(buf) === null;
});

Output of running the above is:

A hello
A he
A
A
{ content: 'A', type: 'A or B', toString: [Function] }
hello w
hel
h

{ content: ' ', type: 'whitespace', toString: [Function] }
hello w
hell
he
h

events.js:72
throw er; // Unhandled 'error' event
^
SyntaxError: could not tokenize "hello world B"

The problem is that for some reason the tokenizer is not matching anything
unless the string starts with h and ends with o. But because disect skips
from hello w to hell it can never match.

The fork that I'm working on completely removes disect because I do not
see how it is even possible to require a perfect match on tokens and yet to
skip some characters.

Thoughts?

—
Reply to this email directly or view it on GitHub
#8 (comment).