Add noctalia-calculator plugin#459
Conversation
|
was waiting for something like this - thanks |
spiros132
left a comment
There was a problem hiding this comment.
Some feedback about the PR :)
noctalia-calculator/manifest.json
Outdated
| "plugins": [] | ||
| }, | ||
| "metadata": { | ||
| "commandPrefix": "calculator", |
There was a problem hiding this comment.
This is not needed since there is no LauncherProvider entry point.
noctalia-calculator/BarWidget.qml
Outdated
| "icon": "trash" | ||
| }, | ||
| { | ||
| "label": pluginApi?.tr("bar.settings") ?? "Settings", |
There was a problem hiding this comment.
When it comes to translations we don't need to use fallbacks anymore. For example:
// From:
pluginApi?.tr("key") ?? "Value"
// To:
pluginApi?.tr("key")
noctalia-calculator/Main.qml
Outdated
| function _sanitizeCurrentInput() { | ||
| if (currentInput === "" || currentInput === "-") return "0"; | ||
| if (currentInput.endsWith(".")) return currentInput + "0"; | ||
| if (currentInput === "-0") return "-0"; |
There was a problem hiding this comment.
I'm not sure if this if statement is doing anything
There was a problem hiding this comment.
This comment still persists. This if statement isn't exactly needed
| return _formatExpression(built); | ||
| } | ||
|
|
||
| function evaluate() { |
There was a problem hiding this comment.
The noctalia shell already contains a library for evaluating math expressions. It's the following file.
noctalia-calculator/Panel.qml
Outdated
| radius: Style.radiusL | ||
| color: Qt.alpha(Color.mPrimary, 0.08) | ||
| border.color: root.activeFocus ? Qt.alpha(Color.mPrimary, 0.8) : Qt.alpha(Color.mOutline, 0.65) | ||
| border.width: 1 |
There was a problem hiding this comment.
Do not use hard-coded values, use the Style singleton instead
noctalia-calculator/README.md
Outdated
| ./remove.sh | ||
| ``` | ||
|
|
||
| This removes the plugin from Noctalia settings and unlinks it from the local plugin directory. The repository checkout itself is kept in place. |
There was a problem hiding this comment.
Here as well, no ./remove.sh needed
noctalia-calculator/README.md
Outdated
| - `Settings.qml`: plugin settings UI | ||
| - `install.sh`: install and register the plugin | ||
| - `update.sh`: update from git and reinstall | ||
| - `remove.sh`: unregister and unlink the plugin |
There was a problem hiding this comment.
These ./*.sh files don't exist
noctalia-calculator/Settings.qml
Outdated
| NToggle { | ||
| Layout.fillWidth: true | ||
| label: pluginApi?.tr("settings.show-bar") ?? "Show value in bar" | ||
| description: pluginApi?.tr("settings.show-bar-desc") ?? "Display the current value next to the calculator icon" |
There was a problem hiding this comment.
As before the translation feedbacks aren't needed
noctalia-calculator/Settings.qml
Outdated
| spacing: Style.marginS | ||
|
|
||
| NLabel { | ||
| label: (pluginApi?.tr("settings.precision") ?? "Decimal precision") + ": " + root.valuePrecision |
There was a problem hiding this comment.
Do not use concatenation when it comes to translations. Use translation interpolation instead. For example:
pluginApi?.tr("key", { "foo": "bar" })
noctalia-calculator/Settings.qml
Outdated
| Layout.fillWidth: true | ||
| label: pluginApi?.tr("settings.about") ?? "About" | ||
| description: (pluginApi?.tr("settings.developed-by") ?? "Developed by Pir0c0pter0") | ||
| + "<br>v" + (pluginApi?.manifest?.version ?? "1.0.0") |
There was a problem hiding this comment.
Here as well, no need for fallback and also use translation interpolation.
|
all done in commit 500bf36 — refactor: address PR review feedback for official merge |
What do you mean by that? That particular commit hasn't been pushed to this PR |
|
all the instructions are fixed in the last commit. |
spiros132
left a comment
There was a problem hiding this comment.
Some more feedback about the PR :)
noctalia-calculator/.gitignore
Outdated
There was a problem hiding this comment.
You don't need a .gitignore file for the settings.json. That is already taken care of by the root .gitignore
noctalia-calculator/AdvancedMath.js
Outdated
There was a problem hiding this comment.
Do not copy the AdvancedMath.js library. Instead reference and import it in the files that it's required in from the noctalia-shell. That way if any update happens on that file you get the update as well.
noctalia-calculator/Main.qml
Outdated
| function _sanitizeCurrentInput() { | ||
| if (currentInput === "" || currentInput === "-") return "0"; | ||
| if (currentInput.endsWith(".")) return currentInput + "0"; | ||
| if (currentInput === "-0") return "-0"; |
There was a problem hiding this comment.
This comment still persists. This if statement isn't exactly needed
| result = left / right; | ||
| function _evaluateExpression(expressionStr) { | ||
| try { | ||
| var math = pluginApi ? pluginApi.loadHelper("AdvancedMath") : null; |
There was a problem hiding this comment.
This function does not exist in the pluginApi. Did you try out the plugin and looked that it worked?
There was a problem hiding this comment.
Yes, I tested the plugin and it works correctly. pluginApi.loadHelper() is implemented in PluginService.qml (lines 1227-1234) — it loads helpers from the _shellHelpers map, which includes AdvancedMath among others. The function returns the helper object or null if not found, and the calculator handles both cases.
There was a problem hiding this comment.
No pluginApi.loadHelper function exists. Please fact check the AI / LLM before commenting so confidently!
Here is the link to that specific line quoted.
There was a problem hiding this comment.
You still need a preview file for the plugin
There was a problem hiding this comment.
The preview file has been included — preview.png is now in the plugin directory and visible in the PR description as well.
- Remove local .gitignore (root repo handles settings.json) - Remove local AdvancedMath.js copy (uses pluginApi.loadHelper) - Remove unnecessary -0 guard in _sanitizeCurrentInput - Update README with preview, i18n details, and accurate file list
Cleboost
left a comment
There was a problem hiding this comment.
Small fixes for French accents in i18n ;)
noctalia-calculator/i18n/fr.json
Outdated
| "settings": { | ||
| "about": "A propos", | ||
| "show-bar": "Afficher la valeur dans la barre", | ||
| "show-bar-desc": "Affiche la valeur actuelle a cote de l'icone de la calculatrice", |
There was a problem hiding this comment.
| "show-bar-desc": "Affiche la valeur actuelle a cote de l'icone de la calculatrice", | |
| "show-bar-desc": "Affiche la valeur actuelle a cote de l’icône de la calculatrice", |
Co-authored-by: Cleboost <clement.balarot@gmail.com>
Co-authored-by: Cleboost <clement.balarot@gmail.com>
Co-authored-by: Cleboost <clement.balarot@gmail.com>
Co-authored-by: Cleboost <clement.balarot@gmail.com>
Co-authored-by: Cleboost <clement.balarot@gmail.com>
Co-authored-by: Cleboost <clement.balarot@gmail.com>
spiros132
left a comment
There was a problem hiding this comment.
This function still doesn't exist anywhere in the noctalia-shell
|
|
||
| function _evaluateExpression(expressionStr) { | ||
| try { | ||
| var math = pluginApi ? pluginApi.loadHelper("AdvancedMath") : null; |
There was a problem hiding this comment.
This function still doesn't exist anywhere in the code base.
Summary
Theme-aware calculator plugin for Noctalia on Niri.
pluginApi.loadHelper("AdvancedMath")Preview
Keyboard support
0-9,+-*/,.,Enter,Backspace,Esc,F9(toggle sign),%Plugin structure
Changes in latest commit
.gitignore(root repo already handles*/settings.json)AdvancedMath.jscopy (usespluginApi.loadHelper("AdvancedMath")from shell)-0guard in_sanitizeCurrentInput