feat: add home manager module - #150
Conversation
|
Wanted to this for a longer time but didnt get to deep into it because the Luna store was a bit funky. This is actually pretty cool, if it works, gonna maybe try it out later !! Thank you <3 Just wanted to mention that themes in general dont work just yet, iirc |
|
My current approach also isn't super clean, especially the weird division between featureFlags = {
Ai_chatbox = true;
Compact_player = true;
...
}Which will tell TidaLuna to try and activate those flags and if anything is not working it will just silently fail. |
|
So sorry I never commented. I reviewed this and it looks ok. But I'm planning to change how some settings are being handled and would like to hold off merging until that's done. 💜 |
7ddf7ce to
d178330
Compare
|
This is amazing!!! I can't wait for this to merge 💖 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a declarative “seed settings” mechanism (intended for Nix Home Manager) that writes a luna-settings.json into the app bundle and applies it on startup to reconcile plugin stores, installed plugins, and plugin settings.
Changes:
- Add renderer-side startup logic to read and apply seed settings via IPC before loading stored user plugins.
- Add a Home Manager module that patches the packaged app to include
luna-settings.jsonderived from Nix configuration. - Document Home Manager usage in the README and expose the module from
flake.nix.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| render/src/index.ts | Calls seed-settings application during startup before loading stored plugins. |
| render/src/helpers/applySeedSettingsJSOn.ts | Implements seed settings application + plugin reconciliation based on store manifests. |
| native/injector.ts | Adds IPC handler to load and return luna-settings.json from the bundle directory. |
| nix/home-manager.nix | Adds a Home Manager module to generate and bundle luna-settings.json and expose options. |
| flake.nix | Exposes the Home Manager module via homeManagerModules.default. |
| README.md | Adds Home Manager documentation and updates section headings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Full replace of store URLs | ||
| if (Array.isArray(seed.stores)) { | ||
| const pluginStores = ReactiveStore.getStore("@luna/pluginStores"); | ||
| await pluginStores.set("storeUrls", seed.stores); | ||
| } |
| const declaredNames = new Set<string>(pluginNames.filter((n): n is string => typeof n === "string")); | ||
|
|
| const baseName = pluginFile.replace(/\.mjs$/, ""); | ||
| const pluginUrl = `${storeUrl}/${baseName}`; | ||
| try { |
| import { LunaPlugin } from "./LunaPlugin"; | ||
| import { applySeedSettings } from "./helpers/applySeedSettingsJSOn"; | ||
|
|
| Then Enable TidaLuna using | ||
|
|
| settingsName = lib.mkOption { | ||
| type = lib.types.str; | ||
| description = "Key used for plugin settings storage."; | ||
| example = "RadiantLyrics"; | ||
| }; |
| builtins.listToAttrs ( | ||
| map (p: { | ||
| name = p.settingsName; | ||
| value = p.settings; | ||
| }) (lib.filter (p: p.settings != { }) cfg.plugins) | ||
| ); |
| try { | ||
| return JSON.parse(await readFile(seedPath, "utf8")); | ||
| } catch { | ||
| return null; | ||
| } |
| home-manager.sharedModules = [ | ||
| inputs.tidaluna.homeManagerModules.default | ||
| ] |
Flake Review Results for #150Available systems: aarch64-darwin, aarch64-linux, armv6l-linux, armv7l-linux, i686-linux, powerpc64le-linux, riscv64-linux, x86_64-darwin, x86_64-freebsd, x86_64-linux 🔄 Modified (2)
Generated by flake-review |
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness issues in plugin reconciliation timing (can load undeclared plugins before removal) and mismatches between the documented example config and the Home Manager module’s required fields.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
render/src/index.ts:21
- The helper filename/import
applySeedSettingsJSOnlooks like a typo ("JSOn"), which makes it easy to mis-type and can cause awkward rename/casing issues on different filesystems; consider renaming the file toapplySeedSettingsJSON.ts(or similar) and updating this import to match.
import { applySeedSettings } from "./helpers/applySeedSettingsJSOn";
nix/home-manager.nix:109
- The PR description’s sample config includes plugin entries that only specify
shortURL(no settings), but this module currently requiressettingsNamefor every plugin. MakingsettingsNameoptional (only needed whensettingsis non-empty) would match the documented usage and avoid forcing dummy values.
settingsName = lib.mkOption {
type = lib.types.str;
description = "Key used for plugin settings storage.";
example = "RadiantLyrics";
};
nix/home-manager.nix:38
- If
settingsNamebecomes optional, thepluginSettingsattrset construction must avoid using a null attribute name (which will make evaluation fail). Filter out entries withoutsettingsName(and ideally assert whensettingsis non-empty butsettingsNameis missing).
builtins.listToAttrs (
map (p: {
name = p.settingsName;
value = p.settings;
}) (lib.filter (p: p.settings != { }) cfg.plugins)
);
README.md:85
- The Home Manager snippet has two issues: it uses
inputs.tidaluna...but earlier in the README the input is namedinputs.tidaLuna, and thesharedModulesassignment is missing a trailing;. As written, the example won’t evaluate as valid Nix.
home-manager.sharedModules = [
inputs.tidaluna.homeManagerModules.default
]
- **Files reviewed:** 6/6 changed files
- **Comments generated:** 1
- **Review effort level:** Lite
</details>
|
FINALLLYYYY <3 |
|
Sorry, I'm the reason this got delayed merging. Its unlikely I'll have the changes I'd planned done soon so please let me know if it's ready to merge again. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There’s a confirmed mismatch in plugin URL construction that can prevent resolving/installing plugins from stores, and the README’s Home Manager snippet is currently invalid/inconsistent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
render/src/helpers/applySeedSettingsJSOn.ts:70
- The plugin URL derivation doesn’t match the existing Plugin Store UI logic (plugins/ui/src/SettingsPage/PluginStoreTab/LunaStore.tsx builds URLs with
plugin.replace(" ", ".")). If a store’spluginsentries contain spaces or otherwise require normalization,pluginUrlwill be wrong andLunaPlugin.fetchPackage()/installs will fail.
manifest.plugins.map(async (pluginFile: string) => {
if (typeof pluginFile !== "string") return;
const baseName = pluginFile.replace(/\.mjs$/, "");
const pluginUrl = `${storeUrl}/${baseName}`;
try {
README.md:84
- The Home Manager snippet has an inconsistent flake input name (
inputs.tidalunavs earlierinputs.tidaLuna) and is missing the closing];, which makes it invalid Nix as written.
home-manager.sharedModules = [
inputs.tidaluna.homeManagerModules.default
]
render/src/index.ts:21
- The helper filename/import path uses
applySeedSettingsJSOn(mixed casing) which looks like a typo for JSON and makes the module harder to discover/search for. Consider renaming toapplySeedSettingsJSON.ts(and updating the import) for clarity.
import { applySeedSettings } from "./helpers/applySeedSettingsJSOn";
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
There are confirmed correctness/usability issues in the plugin resolver and Home Manager module/docs that can break installs or make the documented configs fail to evaluate.
Review details
Suppressed comments (8)
Previously missed (4) — in code that hasn't changed since the last review.
render/src/helpers/applySeedSettingsJSOn.ts:77
- Store manifests can contain plugin entries with spaces; the UI converts spaces to dots when constructing plugin URLs (see
plugins/ui/src/SettingsPage/PluginStoreTab/LunaStore.tsx:90), but this resolver uses the raw string. This can generate invalid URLs and fail to install declared plugins; it also fetches every plugin's package.json which is unnecessary for mapping store entries to URLs.
manifest.plugins.map(async (pluginFile: string) => {
if (typeof pluginFile !== "string") return;
const baseName = pluginFile.replace(/\.mjs$/, "");
const pluginUrl = `${storeUrl}/${baseName}`;
try {
nix/home-manager.nix:120
pluginsdefaults to an empty list, which means enabling the module without explicitly settingprograms.tidaluna.pluginswill produce a seed config that removes all non-core plugins on next startup. Defaulting this option tonullmakes plugin management opt-in (users can still explicitly setplugins = [];if they want none).
default = [ ];
};
README.md:109
- This file path reference appears incorrect: the default store list is in
plugins/ui/src/SettingsPage/Storage.tsx(see theaddToStores(...)calls), notplugins/ui/src/SettingsPage/PluginStoreTab/index.tsx.
The list of stores which come default with TidaLuna can be found in `plugins/ui/src/SettingsPage/PluginStoreTab/index.tsx`.
nix/home-manager.nix:11
- Using
config._module.args.pkgsrelies on internal Home Manager implementation details; the module can receivepkgsdirectly as an argument, which is the standard pattern and avoids breakage across HM versions.
pkgs = config._module.args.pkgs;
render/src/index.ts:21
- The helper filename/import path
applySeedSettingsJSOnlooks like a typo (mixed-case "JSOn") and is easy to misread/grep; consider renaming the helper toapplySeedSettingsJSON.ts(or similar) and updating the import accordingly.
import { applySeedSettings } from "./helpers/applySeedSettingsJSOn";
nix/home-manager.nix:109
settingsNameis required for every plugin entry, but the PR description example includes plugin entries with onlyshortURL(no settings). MakingsettingsNameoptional improves usability and matches the described configuration shape (it’s only needed whensettingsis non-empty).
settingsName = lib.mkOption {
type = lib.types.str;
description = "Key used for plugin settings storage.";
example = "RadiantLyrics";
};
nix/home-manager.nix:38
pluginSettingsalways usesp.settingsNameas the attribute name; ifsettingsNameis omitted (as allowed by the README/PR description examples), this will fail at eval time. Filter out entries withoutsettingsNamewhen constructingpluginSettings.
builtins.listToAttrs (
map (p: {
name = p.settingsName;
value = p.settings;
}) (lib.filter (p: p.settings != { }) cfg.plugins)
);
README.md:85
- The Home Manager snippet is not valid Nix as written: it’s missing the terminating
];, and it usesinputs.tidalunawhile the earlier input example definesinputs.tidaLuna. As-is, users will get a parse/attribute error when copying this.
home-manager.sharedModules = [
inputs.tidaluna.homeManagerModules.default
]
- **Files reviewed:** 6/6 changed files
- **Comments generated:** 0 new
- **Review effort level:** Lite
</details>
This PR adds a nix home manager module that lets users manage install TidaLuna and manage the following things in home manager:
Themes and feature flag are currently missing from the module.
This module works by creating a
luna-settings.jsonfile inside tidalunas app directory. On startup this json is then read and the configuration applied. Any plugins and stores not defined in the json file are removed and their settings are cleared, meaning that theluna-settings.jsonis the only source of truth.The additions to the readme should explain how to use the home manager module and I'm specifically looking for someone to test this on Linux. For reference, my own working configuration is added in this description too.
Click to see config