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

[#12] Fix undefined return of .modules if no monaco plugin is installed. #13

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

Conversation

Gk0Wk
Copy link

@Gk0Wk Gk0Wk commented Mar 26, 2022

No description provided.

Copy link
Owner

@SmilyOrg SmilyOrg left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this! Left a comment :)

Comment on lines +55 to +57
$tw.modules.forEachModuleOfType('monaco', function (title, exportModules) {
// Actually, no need to call require(title)
});
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like you're right. In that case this whole block can be deleted, right?

I copied this from CodeMirror's engine.js assuming it was needed, so I wonder why it was added there? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

See boot.js in tiddlywiki:

$tw.modules.forEachModuleOfType = function(moduleType,callback) {
	var modules = $tw.modules.types[moduleType];
	$tw.utils.each(modules,function(element,title) {
		callback(title,$tw.modules.execute(title));
	});
};

this method has already executed each code of module tiddlers.

Only why CodeMirror adds these codes, I think maybe there is no auto-loading mechanism at the beginning btw, or maybe the codes themselves are unnecessary. The reason why the CodeMirror plugin does not have errors is that the CodeMirror plugin must contain at least one plugin.

I think we should do the same - using the tw-monaco plugin must be used to write tw in the first place, right?

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry for the late response!

I initially tried to model it closer to CodeMirror with features being separate plugins, but then I realized it might just make it harder to install. So I ended up with the main monaco plugin and a monaco-wikitext plugin that adds wikitext-specific functionality.

We can go a little further and just merge all the monaco-wikitext functionality into the main monaco plugin as I'm not sure there is any need for it to be separate anyway. If people only want some parts of it that could be done via plugin configuration.

What do you think?

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