Open khill-fbmc opened 4 months ago
Language file analysis report: | File | Missing Keys | Unused Keys |
---|---|---|---|
en.json | 0 | 2 |
Language file analysis report: | File | Missing Keys | Unused Keys |
---|---|---|---|
en.json | 0 | 2 |
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...
@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.
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.
Language file analysis report: | File | Missing Keys | Unused Keys |
---|---|---|---|
en.json | 0 | 2 |
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.
I just noticed this, maybe I could use it instead of my hard-coded URL
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.
I have not forgotten this. I am just ADHD and things sit longer than I realize :)
@khill-fbmc no problem! Take the time you need for this PR.
Description
This PR adds the following features:
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.Editor
that enables the toggling of the tooltips if you do not want them.Desktop Screenshots
Editor Before
Editor After
Settings Before
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