config: add launched-from to specify launch source#7503
Conversation
Related to #7433 This extracts our "launched from desktop" logic into a config option. The default value is detection using the same logic as before, but now this can be overridden by the user. This also adds the systemd and dbus activation sources from #7433. There are a number of reasons why we decided to do this: 1. It automatically gets us caching since the configuration is only loaded once (per reload, a rare occurrence). 2. It allows us to override the logic when testing. Previously, we had to do more complex environment faking to get the same behavior. 3. It forces exhaustive switches in any desktop handling code, which will make it easier to ensure valid behaviors if we introduce new launch sources (as we are in #7433). 4. It lowers code complexity since callsites don't need to have N `launchedFromX()` checks and can use a single value.
| // do this when the app is launched from the desktop because then | ||
| // we're in an app bundle and we are expected to read from our | ||
| // Bundle's preferred languages. | ||
| if (internal_os.launchedFromDesktop()) language: { |
There was a problem hiding this comment.
I could be wrong, but I spent some time dwelling on this and I don't think we needed this guard. If we launch from a non-desktop source, one of the prior language setting cases should match first. If they don't, we should probably still try this.
| if (self.@"launched-from" == null) { | ||
| self.@"launched-from" = .detect(); | ||
| } |
There was a problem hiding this comment.
I dunno how I feel about this... Maybe we should add a _launched_from variable that is always non-null? I just don't like seeing .? everywhere I guess
There was a problem hiding this comment.
Yeah I felt the same way and thought the same thing, but was worried that it'd be too easy to not know which one to check as a consumer (there being 2 sources of seeming truth). The ? is optimized away in unsafe builds and I think the safety it affords in safe builds is fine.
Related to #7433
This extracts our "launched from desktop" logic into a config option. The default value is detection using the same logic as before, but now this can be overridden by the user.
This also adds the systemd and dbus activation sources from #7433.
There are a number of reasons why we decided to do this:
It automatically gets us caching since the configuration is only loaded once (per reload, a rare occurrence).
It allows us to override the logic when testing. Previously, we had to do more complex environment faking to get the same behavior.
It forces exhaustive switches in any desktop handling code, which will make it easier to ensure valid behaviors if we introduce new launch sources (as we are in linux: add dbus and systemd activation services #7433).
It lowers code complexity since callsites don't need to have N
launchedFromX()checks and can use a single value.