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

Cannot reference grammar from 3rd party plugin #654

Closed
mickaelistria opened this issue Jan 8, 2024 · 6 comments · Fixed by #655
Closed

Cannot reference grammar from 3rd party plugin #654

mickaelistria opened this issue Jan 8, 2024 · 6 comments · Fixed by #655

Comments

@mickaelistria
Copy link
Contributor

mickaelistria commented Jan 8, 2024

In eclipseide-jdtls, we reference the grammar from the TM4E language pack. This was working until 4199ad9 . Now, the same code makes the editor fail at being initialized, with a message assuming it's looking for the grammar inside this current consumer bundle, which is not true.

The mapping should be more relaxed: if a scopeName is set, then instead of failing if not defined locally it could pick any of the defined ones in 3rd party plugins.

@mickaelistria mickaelistria changed the title Allow to reference Cannot reference grammar from 3rd party plugin Jan 8, 2024
mickaelistria added a commit to mickaelistria/textmate.java that referenced this issue Jan 8, 2024
@sebthom
Copy link
Member

sebthom commented Jan 8, 2024

I am not happy with the fix via #655 and will probably revert it.

AbstractGrammarRegistryManager#grammarForScopeName should not throw an exception if a grammar is not found but return null. so the issue lies elsewhere.

with #655 any TMException that may happen during loading a grammar for other reasons than a not found grammar is swallowed and neither reported to the log nor the UI.

Can you please provide a full stacktrace of the exception you got.

@sebthom sebthom reopened this Jan 8, 2024
@sebthom
Copy link
Member

sebthom commented Jan 8, 2024

I could reproduce it. This is not supposed to happen.

org.eclipse.tm4e.core.TMException: No grammar provided for <source.java@some.plugin.id>
	at org.eclipse.tm4e.core.internal.grammar.dependencies.ScopeDependencyProcessor.collectReferencesOfReference(ScopeDependencyProcessor.java:117)
	at org.eclipse.tm4e.core.internal.grammar.dependencies.ScopeDependencyProcessor.processQueue(ScopeDependencyProcessor.java:82)
	at org.eclipse.tm4e.core.registry.Registry._loadGrammar(Registry.java:139)
	at org.eclipse.tm4e.core.registry.Registry.loadGrammar(Registry.java:126)
	at org.eclipse.tm4e.registry.internal.AbstractGrammarRegistryManager.getGrammarForScope(AbstractGrammarRegistryManager.java:198)
	at org.eclipse.tm4e.registry.internal.AbstractGrammarRegistryManager.getGrammarFor(AbstractGrammarRegistryManager.java:168)
	at org.eclipse.tm4e.ui.text.TMPresentationReconciler.findGrammar(TMPresentationReconciler.java:339)
	at org.eclipse.tm4e.ui.text.TMPresentationReconciler$TextViewerListener.inputDocumentChanged(TMPresentationReconciler.java:212)

@mickaelistria
Copy link
Contributor Author

I am not happy with the fix via #655 and will probably revert it.

Any other fix that would still allow referencing 3rd party grammars without adding a provider-specific suffix and that doesn't throw an exception on missing grammar would be fine. This issue was blocker for eclipseide-jdtls and wildwebdeveloper which have referenced the grammars from the language_pack.

@sebthom
Copy link
Member

sebthom commented Jan 8, 2024

I'm looking into it and yes the idea is that you can still reference syntaxes using the plain textmate scope name from other plugins.

@sebthom
Copy link
Member

sebthom commented Jan 8, 2024

I hopefully fixed the root cause now. please try the latest snapshot release.

@mickaelistria
Copy link
Contributor Author

Thank you for this improved fix.

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 a pull request may close this issue.

2 participants