Previously the DisplayLayer would have issues when closing tags that had
not been previously opened, requiring us to guard against mismatched
scopes at the frontier of asynchronous tokenization (see 5cfe9716 for
more information).
Now the DisplayLayer gracefully handles closing tags that had not been
opened, meaning we can eliminate this costly logic.
Signed-off-by: Nathan Sobo <nathan@github.com>
Previously, when calling `TokenizedBufferIterator.seek` with a position
that lied within a text tag, we advanced the iterator by the extent of
that tag without, however, consuming it. Hence, when calling
`moveToSuccessor` afterward, we would consume that tag and advance the
iterator again, thus effectively moving it twice and making its position
inaccurate.
An option could be to clip to the left of the textual tag without
consuming it. However, this would be a little odd with respect to the
current contract between (`DisplayLayer` and) `seek`, whose promise is
to move the iterator to a position that is greater or equal than the one
asked by the caller.
Therefore, with this commit, we are changing the behavior of `seek` in
this particular scenario to consume the tag in question and process all
its siblings until a tag boundary is finally found. This ensures that
the above contract is always respected, while still preserving the "seek
to leftmost tag boundary" semantics (i.e. notice how in the changed test
case, calling `seek` with `Point(0, 1)` is the same as calling it with
`Point(0, 3)`).
Sometimes, when performing an edit, a change on some row can cause
another row's tokenization to be affected: the classic example is
opening a multi-line comment on a line, thereby causing subsequent lines
to become commented out without changing the buffer's contents at those
locations. We call this technique "spill detection".
Since the amount of affected lines can grow quite large, Atom tokenizes
synchronously only those lines where the edit occurred, triggering
background (i.e. `setInterval`) tokenization for all the other lines
that need to be refreshed because of a "spill".
As predictable, this approach causes a temporary inconsistency in the
stored tokenized lines. In particular, suppose we had two tokenized
lines, and that there's an open tag in the middle of the first one which
closes on the second one. If we perform an edit that causes that tag to
be deleted, when reading the second tokenized line we now have a
dangling close tag.
This didn't matter much in the `DisplayBuffer` version, because for each
line we reopened all the tags found in the stored `openScopes` property,
and closed all the tags starting on such line right at the end of it.
In the `DisplayLayer` world, however, we don't read tags from each
tokenized line, but we let `TokenizedBufferIterator` report tag
boundaries and their respective location: since this is an
iterator-based approach, we were not reading `openScopes` for each
`TokenizedLine`, making the dangling close tag example showed above
evident (e.g. close and open tags didn't match anymore, and exceptions
were being thrown all over the place).
To solve this issue I have considered several approaches:
1. Recompute all the lines where a spill occurs synchronously when the
buffer changes. For large files, this can be pretty onerous, and we
don't want to regress in terms of performance.
2. Let `TokenizedBuffer.tokenizedLineForRow(bufferRow)` recompute
potential invalid lines lazily (starting from the first invalid line,
down to the requested buffer row). When editing the first lines of a
long file and causing a spill to occur, Atom (or any other package,
for that matter) could request a line down in the file, causing this
method to recompute lots and lots of lines.
3. Let `DisplayLayer` deal with closing an un-opened tag. This is nice
because we already keep track of containing tags there. However, it
also feels like the wrong spot where to put this logic, as display
layers shouldn't deal with grammar-related stuff.
4. Keep track of containing tags in `TokenizedBufferIterator`, and
report a boundary at the end of the line when the subsequent one's
`openScopes` property doesn't match the `containingTags` that the
iterator has been keeping track of.
Of all these solutions I've chosen 4), because it's the most performant
and clean in terms of code.
If no non-negative integer exists between an open and close tag code,
we still need to report a boundary there, since close tags always have
to come first at any given iterator position.
:pear:ed with @as-cii
