Skip to content

Conversation

@thecrypticace
Copy link
Contributor

This should be mostly functionally equivalent. It simplifies a good chunk of the code used throughout to find class lists and class names.

  1. It eliminates many repeated calls to getConfiguration(…) which would increase based on the number of embedded languages in the document. These hit a cache anyway so the overhead wasn't significant but the repeated calls aren't necessary.
  2. Because of this I've now made several things synchronous where they were async before.
  3. It eliminates a search range limitation for hovers which would cause some class name hovers to fail on large enough documents depending on document structure and what came after the class name further in the document.
  4. This will simplify a future refactor that caches language boundaries, class lists, and class names on a per-document basis and only recompute when documents change.

We replace `findClassNamesInRange` with a two step process we use elsewhere: 1. find all class lists in the document; 2. find classes in each class list
Instead of dropping down to the individual “range” helpers we should test things at the document level since everything should go through those
This was already the case because the class attribute matching was slicing on text with comments already removed.
This simplifies some of the code making it more linearly scannable
This guarantees class hovers work identically to things like color decorators that already use whole document class lists
Document boundaries are handled at a level above this now so we can guarantee that this will only be called for individual document parts — including JSX. Because of this we can simplify how we do class function matching.
This makes the code a bit more linear. Using `.split(…)` is still optimal for performance in V8 and JSC when splitting larger class lists.

This also refactors the code a bit for clarity.
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