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

live-input-configuation #3531

Merged
merged 7 commits into from
Sep 3, 2024
Merged

live-input-configuation #3531

merged 7 commits into from
Sep 3, 2024

Conversation

AlanGriffiths
Copy link
Contributor

@AlanGriffiths AlanGriffiths commented Aug 7, 2024

Allow servers to dynamically update input configuration.

There's a very simple "mir_demo_server.input" addition to mir_demo_server to exercise part of this API, but that is not intended prototype.

@AlanGriffiths
Copy link
Contributor Author

AlanGriffiths commented Aug 7, 2024

This is still not complete:

  • There's no code/tests to exercise this API
  • There's no facility to recover the default or configured configuration

@AlanGriffiths AlanGriffiths mentioned this pull request Aug 9, 2024
@AlanGriffiths AlanGriffiths changed the base branch from main to ReloadingConfigFile August 9, 2024 16:20
@Saviq
Copy link
Collaborator

Saviq commented Aug 20, 2024

Recap of discussion:

  • input configuration should remain in the main .config file
  • we're leaning towards SIGHUP-reloading of the main config file
    • incrementally we'll enable live reinterpretation of the individual options
    • initial target are the input configuration options
    • some might not ever become re-interpretable
  • as today, environment variables and command line takes precedence, basically making those static

@AlanGriffiths
Copy link
Contributor Author

  • input configuration should remain in the main .config file

I don't think that a (hypothetical) input configuration utility should be updating the main .config file

@Saviq
Copy link
Collaborator

Saviq commented Aug 20, 2024

I don't think that a (hypothetical) input configuration utility should be updating the main .config file

Agree, that will have to come from the shell implementation, e.g. integrating gsettings.

@AlanGriffiths
Copy link
Contributor Author

I don't think that a (hypothetical) input configuration utility should be updating the main .config file

Agree, that will have to come from the shell implementation, e.g. integrating gsettings.

Right, so the input configuration needs to be exposed (like this WIP), not "remain in the main .config file"

@Saviq
Copy link
Collaborator

Saviq commented Aug 20, 2024

Right, so the input configuration needs to be exposed (like this WIP), not "remain in the main .config file"

Sorry, different trains of thought. …as opposed to a dedicated file.

Base automatically changed from ReloadingConfigFile to main August 21, 2024 13:39
@AlanGriffiths AlanGriffiths marked this pull request as ready for review August 21, 2024 16:21
@AlanGriffiths AlanGriffiths requested a review from a team as a code owner August 21, 2024 16:21
@Saviq Saviq requested a review from mattkae August 22, 2024 08:05
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor things, but seems like a good change overall! Now we just need to get the keyboard stuff in there too :)

examples/mir_demo_server/server_example.cpp Show resolved Hide resolved
examples/mir_demo_server/server_example.cpp Show resolved Hide resolved
src/miral/input_config.h Outdated Show resolved Hide resolved
src/miral/input_configuration.cpp Show resolved Hide resolved
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be a sensible change form the options that I tested!

@mattkae mattkae enabled auto-merge August 23, 2024 17:27
@mattkae mattkae added this pull request to the merge queue Sep 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 3, 2024
@AlanGriffiths
Copy link
Contributor Author

Seems I'm plagued with #3581 today

@AlanGriffiths AlanGriffiths added this pull request to the merge queue Sep 3, 2024
Merged via the queue into main with commit ef592f8 Sep 3, 2024
37 checks passed
@AlanGriffiths AlanGriffiths deleted the live-input-configuation branch September 3, 2024 13:26
AlanGriffiths added a commit that referenced this pull request Sep 9, 2024
Allow servers to dynamically update input configuration.

There's a very simple "mir_demo_server.input" addition to
`mir_demo_server` to exercise part of this API, but that is not intended
prototype.
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.

3 participants