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

Fix 2699 : filename set at parsing stage + nodes.filename scope + filename in the Root AST node #2700

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

hl037
Copy link

@hl037 hl037 commented Jun 14, 2022

What:

  • Scope node.filename changes so that it's easier to follow how this global variable is changed
  • Root AST node call its parent to get a filename too
  • Visitors gets their this.filename from the root node if not provided in the options

Why:

I am implementing a loader to retrieve all the variables defined in a stylus file (and its imports). The easiest way to do it is to look into Evaluator.global.scope. However, the setup adds builtin globals and filtering is necessary. The best way to filter is to check if the AST node's filename is null. However, the parser does not set the filename of the ast nodes.

fix #2699

While implementing this, I encountered several problems with the nodes.filename global variable :

  • It's hard to track when and if it is restored
  • It can leak to other tests

This is why I added code to precisely mark the scope of the modification, and restore its previous value then.

How:

See commit changes (it's quite short)

Checklist:

  • Documentation N/A
  • Unit Tests
  • Code complete
  • Changelog

I am waiting your review before writing changelogs etc.

@hl037 hl037 changed the title Issue 2699 Fix 2699 : filename set at parsing stage + nodes.filename scope + filename in the Root AST node Jun 14, 2022
@hl037 hl037 mentioned this pull request Jun 19, 2022
4 tasks
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.

filename not set in nodes of the root ast when doing Evaluator.evaluate()
1 participant