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

Safety & Simplicity for Config Files #28

Open
TTWNO opened this issue Oct 18, 2022 · 13 comments
Open

Safety & Simplicity for Config Files #28

TTWNO opened this issue Oct 18, 2022 · 13 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed Input Improvements to the input subsystem
Milestone

Comments

@TTWNO
Copy link
Member

TTWNO commented Oct 18, 2022

At this time, Rust's type system is not being used to verify (before the program starts) that the configuration file is correct.
Error with the JSON are far too common, and kind of hard to diagnose.

This issue contains two jobs:

  1. Simplify the config file to allow the use of non-JSON strings as Odilia events. Currently each event is fairly long due to the verbosity of JSON. -- Simplicity
  2. Verify during the initialization process (before grabbing input devices) that the config file is correct and that all the keybindings will be sent properly through the socket. -- Safety

This is boring work, but should be fairly easy for a beginner to figure out, so I will label it as good first issue.

@TTWNO TTWNO added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed Input Improvements to the input subsystem labels Oct 18, 2022
@TTWNO TTWNO added this to the 1.0 milestone Oct 18, 2022
@albertotirla albertotirla changed the title Saftey & Simplicity for Config Files Safety & Simplicity for Config Files Oct 18, 2022
@TTWNO TTWNO modified the milestones: 1.0, 0.2.0 Oct 19, 2022
@francois-caddet
Copy link
Contributor

francois-caddet commented Feb 26, 2023

I'm asking myself two things:

  • Why do we use json for configuration? I personaly prefer toml/ini files for possibly handwritten configs.
  • for the speech config, in a long term would it be a good idea to configure it with DConf? to keep the last set value by the user when restarting odilia and may be to integrate the configuration of odilia in the setting pannel of the desktop. For this last point, not sure how it is done generaly and neither if it's realy a good way to do it. It's more or less an Android like way to configure. but not realy common in linux an could be complex to maintain.

Edit: sorry I just see we use toml actualy.

Edit2: Does this issue relate to sohkd?

@francois-caddet
Copy link
Contributor

Actualy where is the loading code for the config?I saw a verry simple one in common/configbut not sure if it's the only one. Can anyone help me to start on this issue?

thanks

@TTWNO
Copy link
Member Author

TTWNO commented Feb 26, 2023

We don't currently use any configured values.

Previously, we did at least load the config file, but we weren't using it so I removed it some time ago.

Starting ftom scratch is fine here. Use serde :)

Any config loaded should be added to the Stste struct in odilia/src/state.rs

@francois-caddet
Copy link
Contributor

Ok and so,whatshould be configured?verbosity atleast.
probably all the values ofspeed, pitch, volume...
Should I load config for keybindings also? that should be in the input crate right?

@TTWNO
Copy link
Member Author

TTWNO commented Feb 26, 2023

I'm going to say for now, avoid keybindings. How that's done needs to be completelt rewritten.

@francois-caddet
Copy link
Contributor

I'll stay generic over how we load the config, because I'm not sure all of them have to be placed in a configuration file. e.g.: the pitch/volume/speed are something which could change a lot during runtime so may be placed in DConf stuf in a future improvement step no?

@TTWNO
Copy link
Member Author

TTWNO commented Feb 26, 2023

Yes, that is a reasonable assumptuon.

Even something like the welcome message would be good right now.

@francois-caddet
Copy link
Contributor

Ok, I know a crate which let be generic over whereare the config (a file, env variables, or command params) with priorities between them. you think it's a good idea to go with this? or we avoid adding a new dep for now and stay with a serde serialization no more?

@TTWNO
Copy link
Member Author

TTWNO commented Feb 26, 2023

I'm fine with Odilia having more dependencies. Go ahead.

@francois-caddet
Copy link
Contributor

Ok, I start with this. Do I put yourself to reviewers when it's ready?

@TTWNO
Copy link
Member Author

TTWNO commented Feb 26, 2023

Yes please

@C-Loftus
Copy link
Contributor

C-Loftus commented Mar 1, 2024

Is this issue still relevant? Was looking for good first issues and this seemed to be mainly addressed or not as much an issue anymore in light of the cli support

@petrovvvv
Copy link
Contributor

I'm also wondering if this issue is still relevant. If not, are there other good first issues that are ongoing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed Input Improvements to the input subsystem
Projects
None yet
Development

No branches or pull requests

4 participants