Remove startOnUserLogin from the settings; use OS APIs only
#18530
+170
−95
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
We could also go the other way and remove this setting from settings.json. |
This comment has been minimized.
This comment has been minimized.
|
might be interesting to track "by policy" for showing in ui. 🤔 |
DHowett
changed the title
startOnUserLogin from the settings; use OS APIs only
microsoft-github-policy-service
bot
added
Issue-Bug
|
|
|
For UI consistency you should add a lock icon as on the AI page. |
lhecker
approved these changes
Feb 19, 2025
…IAsyncOperation return to the calling context
…ntainer for this)
|
lock icon |
carlos-zamora
approved these changes
Feb 20, 2025
| } | ||
|
|
||
| _UpdateOverrideSystem(); | ||
| _UpdateHelpText(); |
There was a problem hiding this comment.
So yes! SettingContainer did not support dynamic updates to HelpText!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before we had a Settings UI, we added support for a setting called
startOnUserLogin. It was a boolean, and on startup we would try to yeet the value of that setting into the Windows API responsible for registering us as a startup task.Unfortunately, we failed to take into account a few things.
Users could enable our startup task outside the settings file and we would never know it. We would load up, see that
startOnUserLoginwasfalse, and go disable the task again. 🤦Conversely, if the user disables our task outside the app we can never enable it from inside the app. If an enterprise has configured it either direction, we can't change it either.
The best way forward is to remove it from our settings model and only ever interact with the Windows API.
This pull request replaces
startOnUserLoginwith a rich settings experience that will reflect the current and final state of the task as configured through Windows. Terminal will enable it if it can and display a message if it can't.My first attempt at this PR (which you can read in the commit history) made us try harder to sync the state between the settings model and the OS; we would propagate the disabled state back to the user setting when the task was disabled in the OS or if we failed to enable it when the user asked for it. That was fragile and didn't support reporting the state in the settings UI, and it seems like it would be confusing for a setting to silently turn itself back off anyway...
Closes #12564