Skip to content

Comments

RIDER-133554 ColorScheme: support for global variable in GDScript#457

Closed
plane90 wants to merge 6 commits intoJetBrains:masterfrom
plane90:master
Closed

RIDER-133554 ColorScheme: support for global variable in GDScript#457
plane90 wants to merge 6 commits intoJetBrains:masterfrom
plane90:master

Conversation

@plane90
Copy link
Contributor

@plane90 plane90 commented Dec 20, 2025

  • Add two new color keys for globals:
    Autoload,
    Built-in
  • Add sample tags and entries to the Color Settings page
  • Add new key color definitions to the Darcula color scheme
image

@van800
Copy link
Member

van800 commented Dec 20, 2025

Congrats on the pull request!
Unfortunately I am on vacation without the computer. Please expect review after 27.12.

@van800 van800 self-requested a review December 27, 2025 20:04
}

if (psi.containingFile.isInSdk()) {
val autoload = ProjectAutoloadUtil.findFromAlias(txt, element)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this call is too heavy to be called many times and repeated on every change.

snapshot = ReadAction.compute<Snapshot, RuntimeException> { buildSnapshot() }
}

private fun subscribeToChangesVF() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the scenario for the VFS listener?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially added the VFS listener because I thought .tscn script changes could be reflected before a PSI commit (I observed VFS events even without explicitly saving), so I tried to refresh the autoload snapshot when watched .tscn files change.

After re-checking the actual workflow, that observation was based on a wrong assumption on my side: the changes weren’t truly saved/committed yet. The correct behavior should be that updates are applied only after the file is saved and PSI is committed. In that case, the PSI listener (with performWhenAllCommitted) is sufficient, and the VFS listener is unnecessary and potentially misleading.

Sorry for the confusion and for introducing this bug-prone logic. I’ll update the PR to remove (or significantly narrow) the VFS listener and rely on PSI commit-based refresh only.

Additionally, I noticed a bug: if an autoload entry points to a .tscn and its nodes have no script (or the script is cleared), findGdFileForTscnFile returns null and the loop exits early, which prevents collecting the .tscn into watchedFiles.

I will fix this by registering .tscn files in watchedFiles regardless of script resolution, and by falling back to using the .tscn PSI file when no script is present.

@van800
Copy link
Member

van800 commented Jan 16, 2026

Please incorporate (cherry-pick) this change to the pull request
61130fc
and try if it does everything you wanted.

@van800
Copy link
Member

van800 commented Jan 23, 2026

Hi @plane90!
I think it is already almost done.
I can take over this work if you lost interest in finishing.

@plane90
Copy link
Contributor Author

plane90 commented Jan 23, 2026

@van800
It was a pleasure working on this, and I hope my changes are useful. Feel free to take over and complete it as you see fit.

@van800
Copy link
Member

van800 commented Jan 23, 2026

Thank you!

@van800 van800 closed this Jan 23, 2026
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