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

🧹 Remove global variables from syntax highlighter #6085

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

rix0rrr
Copy link
Collaborator

@rix0rrr rix0rrr commented Jan 4, 2025

The syntax highlighter used to work by calling initTranslations() with the current level and language, which then manipulates global variables so that the syntax highlighter code has the right information to highlight the different levels and languages of Hedy correctly.

To remove the need for global variables, we instead parameterize the Lezer parsers and pass the level and language into the generator at every time it is used.

The old parser code looked like this:

import {extendKeyword} from "./tokens"
export const parser = LRParser.deserialize({
  // ...

  specialized: [{term: 12, get: (value: any, stack: any) => (extendKeyword(value, stack) << 1) | 1, external: extendKeyword, extend: true}],
  //                                                         ^^^^^^ depends on language and level
});

extendKeyword depends on language and level, and so what we used to do is put the "current" language and level in a global variable and read the global variable in extendKeyword. This creates a sequencing requirement via global state, and apparently also a race condition because sometimes the syntax highlighters didn't load properly (see #6024).

In this PR, I change the generated source files to look like this:

import {specializeKeywordGen, extendKeywordGen} from "./tokens"
export function generateParser(level: number, language: string): LRParser {
  const extendKeyword = extendKeywordGen(level, language);
  // ^^^^This generates an 'extendKeyword' function that knows about leven and language

  return LRParser.deserialize({
    // ...
    specialized: [{term: 12, get: (value: any, stack: any) => (extendKeyword(value, stack) << 1) | 1, external: extendKeyword, extend: true}],
    //                                                         ^^^^ has the right level and language baked in already
})
}

This PR is a cleanup task from another larger PR I'm doing, that I'd like to separate out.

It also happens to fix #6024 😉

How to test

This shouldn't change anything.

The syntax highlighter used to work by calling `initTranslations()` with
the current level and language, which then manipulates global variables
so that the syntax highlighter code has the right information to
highlight the different levels and languages of Hedy correctly.

To remove the need for global variables, we instead parameterize the
Lezer parsers and pass the level and language into the generator at
every time it is used.
@rix0rrr rix0rrr requested a review from jpelay January 4, 2025 19:55
@rix0rrr rix0rrr marked this pull request as draft January 4, 2025 19:57
@rix0rrr rix0rrr marked this pull request as ready for review January 5, 2025 11:07
Copy link
Contributor

mergify bot commented Jan 7, 2025

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

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.

🪲 Sometimes code is not highlighted (must be because of a js error)
2 participants