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

feat: CodeMirror plugin for Klipper Documentation tooltip links in editor #1898

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

khill-fbmc
Copy link

@khill-fbmc khill-fbmc commented Jun 5, 2024

Description

This PR adds the following features:

  • While in the editor for a printer.cfg file, if you hover over a section heading, like [printer] you will get native CodeMirror tooltips, with a direct link to the corresponding section in the Klipper Documentation.
  • A new settings entry for Editor that enables the toggling of the tooltips if you do not want them.

Desktop Screenshots

Editor Before

editor_before

Editor After

editor_after

Settings Before

settings_before

Settings After

settings_after

Note

I am not the happiest with how the actual code is written for the CodeMirror plugin in CodeMirrorPluginKlipperDocsTooltips.ts, so this PR is the POC / MVP of the idea to see if it has any interest.

PS: I am also terrible with naming files, I apologize that it is so long, heh

Copy link
Contributor

github-actions bot commented Jun 5, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 2

Copy link
Contributor

github-actions bot commented Jun 5, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 2

@khill-fbmc khill-fbmc marked this pull request as ready for review June 5, 2024 19:46
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 5, 2024
@khill-fbmc khill-fbmc marked this pull request as draft June 5, 2024 19:46
@khill-fbmc
Copy link
Author

I have never tried to submit a PR like this to a popular repo, so please be kind if I am not doing it correctly.

I didn't mean to set it for review, unless, that was correct. I will leave this as a draft for now since I assume it might need more work...

@meteyou
Copy link
Member

meteyou commented Jun 5, 2024

@khill-fbmc thank you very much for this PR! I think this is a very useful function for the editor!

i've only looked at it briefly so far, but what immediately caught my eye is the long list of possible Klipper modules. would it be possible to make this list “dynamic”? i.e. simply link to the Klipper docs in general, even if this module doesn't exist?

this would just minimize the effort to update this list.

@khill-fbmc
Copy link
Author

Thank you! I agree that trying to maintain a list of module names was not optimal. I had ChatGPT extract them from the docs page just to see if it would work.

I will work out a better solution and remove the big array of keywords. My thought process was to avoid having any tooltip generate a link that might be a 404.

Copy link
Contributor

github-actions bot commented Jun 5, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 2

@khill-fbmc
Copy link
Author

I removed the keywords array and tweaked the algorithm to match on words in the brackets. I think it is safe that if it is a valid config section, then it will link. If you put [exduder] then a 404 would actually help you realize a typo.

I tweaked the CSS a little too to reduce the amount of contrast with the link text.
image

@kevinkhill
Copy link
Contributor

I just noticed this, maybe I could use it instead of my hard-coded URL

@meteyou
Copy link
Member

meteyou commented Jun 9, 2024

i had a little more time today to take a closer look at the PR and i noticed the following things:

  • i think this would be a good idea, to use the same getter with the languages for the URL.

  • i would not use the word “plugin” in the plugin name, as it is already in the “plugin” folder. this would shorten the name a little (although not much). (before: CodeMirrorPluginKlipperDocsTooltips, after: CodemirrorKlipperDocsTooltips)

  • you have to add the default value for the gui-settings here: https://github.com/mainsail-crew/mainsail/blob/develop/src/store/gui/index.ts#L114 (i would enable this feature per default) and add the type here: https://github.com/mainsail-crew/mainsail/blob/develop/src/store/gui/types.ts

  • the output in the tooltip have to be added to the translation/i18n. i would use a translation string like View {modul} documentation and would add the complete link + name here. this would help translators here, when they need a different sentence position/order.

  • lastly, I'm not sure if you need to design the tooltip differently for light/dark theme

Thank you very much and feel free to ping me, if you need help.

@kevinkhill
Copy link
Contributor

I have not forgotten this. I am just ADHD and things sit longer than I realize :)

@meteyou
Copy link
Member

meteyou commented Jun 25, 2024

@khill-fbmc no problem! Take the time you need for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants