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

CodeMirror 6 editor: Fix Quick Links conflict with other autocomplete plugins #17

Merged

Conversation

personalizedrefrigerator
Copy link
Contributor

Summary

Updates Quick Links to use joplinExtensions.completionSource for autocomplete. This fixes conflicts with the CodeMirror 6 Snippets plugin (and other future autocomplete plugins) without changing this plugin's behavior.

Fixes personalizedrefrigerator/joplin-plugin-codemirror-6-snippets#1.

Testing

  1. Add Quick Links as a development plugin and CodeMirror 6 Snippets
  2. Set up CodeMirror 6 Snippets using a snippets note.
  3. Add a link using Quick Links.
  4. CodeMirror 6 snippets to fill a snippet -- verify that it works and uses the same autocomplete window as Quick Links.
  5. Create a JavaScript code block, type @@function and verify that the CodeMirror builtin completion for function can be used.
  6. Verify that CodeMirror 6 snippets can also be used in the code block.
Screencast_20240227_084823-1.webm

@@ -93,10 +94,22 @@ export default function codeMirror6Plugin(pluginContext: PluginContext, CodeMirr
};
};

let extension: Extension;

if (CodeMirror.joplinExtensions) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CodeMirror.joplinExtensions requires Joplin 2.14.16 or greater.

Copy link
Owner

Choose a reason for hiding this comment

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

So probably worth updating app_min_version in the manifest?

Copy link
Owner

@roman-r-m roman-r-m Mar 2, 2024

Choose a reason for hiding this comment

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

Or I guess it will take the other branch and keep working as before. Except previously there was activateOnTyping: true -- not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the documentation again, it seems that activateOnTypingg defaults to true. As such, it shouldn't be needed in either case.

Or I guess it will take the other branch and keep working as before.

The goal of having two branches is to continue supporting older versions of Joplin. To make this clear, I should include a comment explaining the purpose of the if statement.

Copy link
Owner

Choose a reason for hiding this comment

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

The goal of having two branches is to continue supporting older versions of Joplin. To make this clear, I should include a comment explaining the purpose of the if statement.

No it's clear enough, I should have looked closer before sending that comment. All good. Thanks for the PR.

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.

Conflicts with the Quick Links plugin
2 participants