starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

If we fail to open the settings file, display a message if it existed #1757

New Issue
Open
opened 2026-01-30 22:35:42 +00:00 by claunia · 10 comments
claunia commented 2026-01-30 22:35:42 +00:00
Owner
Copy Link

Originally created by @zadjii-msft on GitHub (Jun 19, 2019).

A follow up from #1325.

In #1325's case, the file did exist, but we failed to open it. In this case, we should display some sort of error dialog to the user, instead of blowing away the user settings with the defaults.

Originally created by @zadjii-msft on GitHub (Jun 19, 2019). A follow up from #1325. In #1325's case, the file _did_ exist, but we failed to open it. In this case, we should display some sort of error dialog to the user, instead of blowing away the user settings with the defaults.
claunia added the Issue-TaskProduct-TerminalArea-UserInterface labels 2026-01-30 22:35:42 +00:00
claunia commented 2026-01-30 22:35:43 +00:00
Author
Owner
Copy Link

@jj10133 commented on GitHub (Sep 18, 2019):

Hi Sir @zadjii-msft I would like to ask you there's a method ValidateProfilesExist() and ValidateDefaultProfileExists() which sets the default profile even if there's no profile. So I would like to ask you that do i make a separate method to show up the dialog or change this method.

@jj10133 commented on GitHub (Sep 18, 2019): Hi Sir @zadjii-msft I would like to ask you there's a method ValidateProfilesExist() and ValidateDefaultProfileExists() which sets the default profile even if there's no profile. So I would like to ask you that do i make a separate method to show up the dialog or change this method.
claunia commented 2026-01-30 22:35:43 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Sep 18, 2019):

If I were going to implement this, I'd honestly not touch either of those. CascadiaSettings::_ReadFile is the method that reads the settings file - I'd first figure out what HRESULT is thrown for a failure to read the file. Then, I'd modify App::_TryLoadSettings() to catch that exception, and check the HRESULT it catches. I believe right now, if it catches any HRESULT I think it assumes that's a JSON parse exception, but it won't always be. I'd add code to handle that exception specifically, and display a different message. You'd need to add another string to Resources.resw as well, so the message could be localized.

@zadjii-msft commented on GitHub (Sep 18, 2019): If I were going to implement this, I'd honestly not touch either of those. `CascadiaSettings::_ReadFile` is the method that reads the settings file - I'd first figure out what `HRESULT` is thrown for a failure to read the file. Then, I'd modify `App::_TryLoadSettings()` to catch that exception, and check the `HRESULT` it catches. I believe right now, if it catches _any_ `HRESULT` I think it assumes that's a JSON parse exception, but it won't always be. I'd add code to handle that exception specifically, and display a different message. You'd need to add another string to `Resources.resw` as well, so the message could be localized.
claunia commented 2026-01-30 22:35:43 +00:00
Author
Owner
Copy Link

@jj10133 commented on GitHub (Sep 19, 2019):

@zadjii-msft CascadiaSettings::_ReadFile is not there but i think you're referring to CascadiaSettings::_ReadSettings() ??

@jj10133 commented on GitHub (Sep 19, 2019): @zadjii-msft CascadiaSettings::_ReadFile is not there but i think you're referring to CascadiaSettings::_ReadSettings() ??
claunia commented 2026-01-30 22:35:43 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Sep 19, 2019):

You might want to fetch and pull 😉

5a57c5a6c9/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp (L730-L745)

@zadjii-msft commented on GitHub (Sep 19, 2019): You might want to fetch and pull 😉 https://github.com/microsoft/terminal/blob/5a57c5a6c975592c2ce52ce078082a209fde5e88/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp#L730-L745
claunia commented 2026-01-30 22:35:43 +00:00
Author
Owner
Copy Link

@jj10133 commented on GitHub (Sep 20, 2019):

@zadjii-msft I did try to remove the json file from roaming path in order to get an error but it creates a json file so i cant get the error code for HRESULT

@jj10133 commented on GitHub (Sep 20, 2019): @zadjii-msft I did try to remove the json file from roaming path in order to get an error but it creates a json file so i cant get the error code for HRESULT
claunia commented 2026-01-30 22:35:44 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Sep 20, 2019):

I'm not 100% sure how to create the scenario that prompted this issue TBH. This was more filed as a "we believe this can be an issue, but we're not sure how yet".

Maybe someone creates a version of the profiles.json file that can only be opened as an Admin or something, and the Terminal ends up not having the proper permissions to access the file. That's the situation that I'm thinking of off the top of my head.

We do in fact create the settings file whenever it doesn't exist, that's an expected scenario. The unexpected scenario is when the file does exist, and we're unable to open it for some reason.

@zadjii-msft commented on GitHub (Sep 20, 2019): I'm not 100% sure how to create the scenario that prompted this issue TBH. This was more filed as a "we believe this _can_ be an issue, but we're not sure how yet". Maybe someone creates a version of the `profiles.json` file that can only be opened as an Admin or something, and the Terminal ends up not having the proper permissions to access the file. That's the situation that I'm thinking of off the top of my head. We do in fact create the settings file whenever it doesn't exist, that's an expected scenario. The unexpected scenario is when the file _does_ exist, and we're unable to open it for some reason.
claunia commented 2026-01-30 22:35:44 +00:00
Author
Owner
Copy Link

@jj10133 commented on GitHub (Sep 21, 2019):

@zadjii-msft I think you there's only one scenario where by the file is empty, i don't think there's any other area.

@jj10133 commented on GitHub (Sep 21, 2019): @zadjii-msft I think you there's only one scenario where by the file is empty, i don't think there's any other area.
claunia commented 2026-01-30 22:35:44 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Sep 21, 2019):

Did you try putting bad permissions on the file? There is more than just those scenarios.

@DHowett-MSFT commented on GitHub (Sep 21, 2019): Did you try putting bad permissions on the file? There is more than just those scenarios.
claunia commented 2026-01-30 22:35:44 +00:00
Author
Owner
Copy Link

@jj10133 commented on GitHub (Sep 21, 2019):

Sir, I don't really understand by permission, is only meant to be opened by the admin?

@jj10133 commented on GitHub (Sep 21, 2019): Sir, I don't really understand by permission, is only meant to be opened by the admin?
claunia commented 2026-01-30 22:35:45 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Aug 3, 2022):

2022 update: we did this for read-only settings files in

but also, I'm not sure this has ever really been an issue. Next time we do a bug sweep, we may want to just clean this one out.

@zadjii-msft commented on GitHub (Aug 3, 2022): _2022 update_: we did this for read-only settings files in * #7727 * #7950 but also, I'm not sure this has ever _really_ been an issue. Next time we do a bug sweep, we may want to just clean this one out.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-UserInterface Issue-Task Product-Terminal
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#1757
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?