Skip to content

Introduce fallback to file based config when dbus not available - #358

Open
catap wants to merge 2 commits into
htkhiem:mainfrom
catap:optional-dbus-dconf
Open

catap wants to merge 2 commits into
htkhiem:mainfrom
catap:optional-dbus-dconf

Conversation

@catap

@catap catap commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes: #352

@catap

catap commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thus, I had run rust fmt, so it fixed some whitespaces.

@htkhiem

htkhiem commented Sep 12, 2026

Copy link
Copy Markdown
Owner

I'm not sure what's going on but looks like all the indentations are gone and there are blank lines between every code line. Can you reformat them first? It's a bit hard to review with the diffs looking like that.

@catap

catap commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

I just run rust-fmt on that files. Attempt to use cargo fails, before I go to do it manually, can you suggest which way you keep your files well formated?

If you don't, well, when I'll recover everything by hand :-)

@htkhiem

htkhiem commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Just wanna confirm, but how did cargo fmt fail on your side?

I'm thinking of just running that once on main first so when you do the same they'll line up & the diffs should be cleaner :)

EDIT: as for how I keep my sources clean, recently I've set it up such that my editor will automatically run rust-fmt upon saving a file. Some old files haven't been touched lately and so are still a bit off, hence my above proposal to just do one full sweep now.

@catap
catap force-pushed the optional-dbus-dconf branch from d4dd567 to 7ebdd98 Compare September 13, 2026 10:26
@catap

catap commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

I created dedicated #359 where I keep how cargo fmt reformat the code. After that I've rebased this PR to that root and it makes the second commit (related to dconf) without reformating.

@htkhiem

htkhiem commented Sep 19, 2026

Copy link
Copy Markdown
Owner

I created dedicated #359 where I keep how cargo fmt reformat the code. After that I've rebased this PR to that root and it makes the second commit (related to dconf) without reformating.

Not sure I'm following here. Shouldn't this be cargo fmt'd too to line up with that root (i.e., main, since I've just merged #359)? As things stand the diffs still look weird.

@htkhiem

htkhiem commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Anyway, I do have a few early remarks (please bear with me, as this is a rather deep-cutting PR):

  • File-based config is welcome as I'm actually looking for one too to provide settings import/export functionality, but
  • The way this is being implemented can probably be "standardised". See SettingsBackend: instead of implementing a new custom settings reader class and having to change imports on the caller side, you might find it easier to implement that interface. Then, on boot-up, check whether D-Bus is available (can be a static Lazy bool flag so it's only ever run once), and if not, use the file-based backend instead of the default one (check that bool in the fn settings_manager() function and get the corresponding backend). That way no caller-side code has to change at all & all new code can be in a single file :).
  • Why are you using INI? I thought with how Euphonica's settings schema has nesting (all those child schemas), something like JSON is better? Also, if you use something like JSON, you can make use of serde to lessen the pain. Don't use YAML though.
  • Completely optional, but may I ask why are you not using D-Bus? (just trying to understand the customer base that is).

@catap

catap commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

@htkhiem I had split this code into two stages to make it easy to review when the fmt PR was revieved. Now I had rebased it and the first commit had gone. Sorry for noise.

Meanwhile, I will address your remarks soon. Thanks for review. And no, I don't against json or anything, but init was simpler choise.

@catap

catap commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Regarding D-Bus: mainly because I can't imagen setup when D-Bus is available and gconf is not. I run it in without any Desktop Environment and my small stumpwm setup works well without D-Bus and because of that I've added file based config in a way which makes releativly happy :-)

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.

Update euphonica fron 0.99.6 to 0.99.7.1 leads to not saving config

2 participants