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

feature: provide a way to specify an additional data source for configuration data #3478

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattkae
Copy link
Contributor

@mattkae mattkae commented Jul 12, 2024

Why?

In miracle (and probably other compositors), people may want to specify configuration options in a different format (e.g. a json file, a yaml file, etc.). To facilitate this, users should be able to specify a simple map that contains the key-value pairs that were parsed from another source.

What's new?

  • Added a new class to miral called ConfigurationDataSource
  • Added DefaultConfiguration::set_options_map (maybe this should be in the constructor, but the constructors were a bit large)
  • Added DefaultConfiguration::parse_custom
  • Added ProgramOption::parse_map
  • Fixed the symbols map generator

To test

In your main runner function, do something like:

return runner.run_with(
        {
            ...
            ConfigurationDataSource([]()
            {
                return std::map<std::string, std::string>{
                    {"startup-apps", "gedit"}
                };
            }),
            ...
         });

@mattkae mattkae requested a review from a team as a code owner July 12, 2024 18:59
@mattkae mattkae changed the title feature: provide an extra data source for configuration data feature: provide a way to specify an additional data source for configuration data Jul 12, 2024
@mattkae mattkae force-pushed the feature/configuration_data_source branch from 500140c to 115394d Compare July 12, 2024 18:59
@mattkae mattkae requested a review from AlanGriffiths July 12, 2024 19:00
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

Attention: Patch coverage is 19.23077% with 21 lines in your changes missing coverage. Please review.

Project coverage is 76.94%. Comparing base (5557be6) to head (115394d).

Files Patch % Lines
src/platform/options/program_option.cpp 0.00% 9 Missing ⚠️
src/miral/configuration_data_source.cpp 0.00% 5 Missing ⚠️
src/server/server.cpp 33.33% 4 Missing ⚠️
src/platform/options/default_configuration.cpp 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3478      +/-   ##
==========================================
- Coverage   76.95%   76.94%   -0.02%     
==========================================
  Files        1080     1081       +1     
  Lines       69380    69405      +25     
==========================================
+ Hits        53394    53403       +9     
- Misses      15986    16002      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlanGriffiths
Copy link
Collaborator

In miracle (and probably other compositors), people may want to specify configuration options in a different format (e.g. a json file, a yaml file, etc.). To facilitate this, users should be able to specify a simple map that contains the key-value pairs that were parsed from another source.

I'm not convinced this is useful in this form.

This adds complexity to the current "configuration fixed at startup" options. As you know, I'm working to replace a lot of what "configuration fixed at startup" is currently used for with a way to dynamic configuration changes whilst running. I suspect a lot of what you want to source from "a json file, a yaml file, etc." is going to fall into the "dynamic configuration" category and is better built on that work.

@mattkae
Copy link
Contributor Author

mattkae commented Jul 15, 2024

I'm not convinced this is useful in this form.

This adds complexity to the current "configuration fixed at startup" options. As you know, I'm working to replace a lot of what "configuration fixed at startup" is currently used for with a way to dynamic configuration changes whilst running. I suspect a lot of what you want to source from "a json file, a yaml file, etc." is going to fall into the "dynamic configuration" category and is better built on that work.

So long as I can set a Mir-defined configuration option from some external source (e.g. somehow set "--display-config" from a JSON file), I'd be happy. If that work will enable it, then I will close this one in favor 👍

@tarek-y-ismail tarek-y-ismail mentioned this pull request Aug 9, 2024
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.

2 participants