Add E (end of WORD), and fix up e (end of word).

[tma-isbx]

No description provided.

[johnfn]

Hey @tma-isbx! This is looking pretty good, but one thing that I noticed is that in a case like this:

word word word|

word

With | being the cursor, if I press e, the cursor doesn't make it to the next word and actually the extension crashes! I left you a comment where I think this happens. Could you fix that case and add a test for it? Thanks!

[johnfn]

Here we get into an infinite loop.

[tma-isbx]

Fixed infinite loop occurring with w/b/e when trying to traverse empty lines.

Also made sure to match vim's behavior.

w/W stops on empty lines, but skips over whitespace only lines.
b/B stops on empty lines, but skips over whitespace only lines.
e/E skips over both empty lines and whitespace only lines.

[johnfn]

Nice! Those are some bizarre edge cases...

You've got me wondering though. Your code seems a little complicated. The way I'd write this for word ends might look something like this (this is pseudocode, btw)

for (let line = currentLine(); line < maxLines; line++) {
    for (position of wordRegex.exec(currentLine.getText()).allPositions()) {
        if (position > currentPosition) { return position; }
    }
}

return endOfDocument();

It seems this could easily be generalized to all of wWeEbB. Is there any reason why such an approach wouldn't work here?

[tma-isbx]

I've cleaned it up a little bit. It is fundamentally along the same lines as the psuedo code.

Fixed a bug in the regex that required the previous guards against empty lines (so was able to remove those). But still have the special case logic to stop on empty lines.

[johnfn]

Hey @tma-isbx , check this out: 131bf4f

I was able to clean up your code a little bit by incorporating the empty line case in the regex that you use for matching. I also tightened up the outer loop by making it loop directly over current line rather than keeping track of that separately. Finally, I decomposed out the bit of code that turns regexes into positions that match because you use that in a couple different places.

Could you finish off the work I've done (I only did it for the 'w' motion so far) and then commit that? Then I think we can merge!

[tma-isbx]

Looks good. I think it could be directly applied to wordLeft, but I think the regex change breaks e/E which doesn't stop on empty lines (maybe not breaks but e/E would probably need some special handling). And another version of allPositions will be needed for e/E (though it may be possible to use it with just different regexes -- something based off of \S\s+, though there are probably some corner cases there).

I'll take a look.

[johnfn]

Yeah, the way I see it we could just use separate regex rules for the different motions - make e/E the base regex and then add ^$ onto it and make it the w/W regex. Ideally the regex could actually match the end of the word in the e/E case vs the beginning of the word in the w/W case. What's wrong with allPositions in the e/E case? Or is it the same issue?

[tma-isbx]

Current implementation of e/E uses the same word/WORD regexes but uses match.position + match.length as the returned position to find the end of word/WORD.

[tma-isbx]

Updated.

[johnfn]

Ah, beautiful! :)