[PR #4961] [MERGED] Gracefully handle json data with the wrong value types #26053

Open
opened 2026-01-31 09:13:38 +00:00 by claunia · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/4961
Author: @zadjii-msft
Created: 3/17/2020
Status: Merged
Merged: 3/20/2020
Merged by: @undefined

Base: masterHead: dev/migrie/b/4299-wrong-settings-types


📝 Commits (5)

  • 9cb0bd2 Well this makes this particular exception go away
  • 66ba77d mad with templates
  • c83d4d7 add tests
  • a7a540c Add some doc comments
  • 50ea9f1 Merge remote-tracking branch 'origin/master' into dev/migrie/b/4299-wrong-settings-types

📊 Changes

6 files changed (+248 additions, -106 deletions)

View changed files

📝 src/cascadia/TerminalApp/CascadiaSettings.h (+2 -0)
📝 src/cascadia/TerminalApp/GlobalAppSettings.cpp (+15 -36)
📝 src/cascadia/TerminalApp/JsonUtils.cpp (+67 -1)
📝 src/cascadia/TerminalApp/JsonUtils.h (+77 -2)
📝 src/cascadia/TerminalApp/Profile.cpp (+24 -61)
📝 src/cascadia/ut_app/JsonTests.cpp (+63 -6)

📄 Description

Summary of the Pull Request

Currently, if the Terminal attempts to parse a setting that should be a bool
and the user provided a string, then we'll throw an exception while parsing the
settings, and display an error message that's pretty unrelated to the actual
problem.

The same goes for bools as ints, floats as ints, etc.

This PR instead updates our settings parsing to ensure that we check the type of
a json value before actually trying to get its parsed value.

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

I made a bunch of JsonUtils helpers for this in the same vein as the
GetOptionalValue ones.

Notably, any other value type can safely be treated as a string value.

Validation Steps Performed

  • added tests
  • ran the Terminal and verified we can parse settings with the wrong types

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/4961 **Author:** [@zadjii-msft](https://github.com/zadjii-msft) **Created:** 3/17/2020 **Status:** ✅ Merged **Merged:** 3/20/2020 **Merged by:** [@undefined](undefined) **Base:** `master` ← **Head:** `dev/migrie/b/4299-wrong-settings-types` --- ### 📝 Commits (5) - [`9cb0bd2`](https://github.com/microsoft/terminal/commit/9cb0bd2b21425c2f1ef384cfad9dc1f9087fa13b) Well this makes this particular exception go away - [`66ba77d`](https://github.com/microsoft/terminal/commit/66ba77d5ac88df930a136ce002ed17b8b5645a31) mad with templates - [`c83d4d7`](https://github.com/microsoft/terminal/commit/c83d4d7d6a42c89e125db15e275b763735fb0393) add tests - [`a7a540c`](https://github.com/microsoft/terminal/commit/a7a540c2075741c33e122ac952836fd2a798f220) Add some doc comments - [`50ea9f1`](https://github.com/microsoft/terminal/commit/50ea9f158d42d8658fceae67137ce8e2a4d2030b) Merge remote-tracking branch 'origin/master' into dev/migrie/b/4299-wrong-settings-types ### 📊 Changes **6 files changed** (+248 additions, -106 deletions) <details> <summary>View changed files</summary> 📝 `src/cascadia/TerminalApp/CascadiaSettings.h` (+2 -0) 📝 `src/cascadia/TerminalApp/GlobalAppSettings.cpp` (+15 -36) 📝 `src/cascadia/TerminalApp/JsonUtils.cpp` (+67 -1) 📝 `src/cascadia/TerminalApp/JsonUtils.h` (+77 -2) 📝 `src/cascadia/TerminalApp/Profile.cpp` (+24 -61) 📝 `src/cascadia/ut_app/JsonTests.cpp` (+63 -6) </details> ### 📄 Description ## Summary of the Pull Request Currently, if the Terminal attempts to parse a setting that _should_ be a `bool` and the user provided a string, then we'll throw an exception while parsing the settings, and display an error message that's pretty unrelated to the actual problem. The same goes for `bool`s as `int`s, `float`s as `int`s, etc. This PR instead updates our settings parsing to ensure that we check the type of a json value before actually trying to get its parsed value. ## References ## PR Checklist * [x] Closes #4299 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments I made a bunch of `JsonUtils` helpers for this in the same vein as the `GetOptionalValue` ones. Notably, any other value type can safely be treated as a string value. ## Validation Steps Performed * added tests * ran the Terminal and verified we can parse settings with the wrong types --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:13:38 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#26053