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

Use .rc files in TSM instead of string literals #16844

Merged
merged 14 commits into from
Mar 14, 2024

Conversation

PankajBhojwani
Copy link
Contributor

@PankajBhojwani PankajBhojwani commented Mar 8, 2024

More prerequisite work for Action IDs - turns out if we add the action IDs to the actions defined in defaults.json the string ends up being too large and the compiler complains about it. Use a .rc file for defaults.json instead and also for enableColorSelection.json + userDefaults.json.

@PankajBhojwani PankajBhojwani marked this pull request as draft March 8, 2024 02:03
@PankajBhojwani PankajBhojwani marked this pull request as ready for review March 9, 2024 00:14
@PankajBhojwani
Copy link
Contributor Author

PankajBhojwani commented Mar 9, 2024

For some reason the tests are unable to load the settings the new way, they throw this exception:
Caught std::exception: Exception while loading or validating Terminal settings

@DHowett
Copy link
Member

DHowett commented Mar 11, 2024

For some reason the tests are unable to load the settings the new way, they throw this exception: Caught std::exception: Exception while loading or validating Terminal settings

Remember! Settings Tests is a separate DLL that consumes the Settings static library. It doesn't know about the .res file, just like Settings DLL didn't know.

tools/CompressJson.ps1 Outdated Show resolved Hide resolved
@DHowett
Copy link
Member

DHowett commented Mar 11, 2024

nit: make sure your PR title is short (72 characters is a great maximum) and descriptive, and that the body is up to date and complete.

@DHowett
Copy link
Member

DHowett commented Mar 11, 2024

If you want to follow the style for my commit messages, you can remove the headings and the empty checkboxes.

@DHowett
Copy link
Member

DHowett commented Mar 11, 2024

(and final nit: it's not instead of powershell scripts, it's instead of string literals!)

@PankajBhojwani PankajBhojwani changed the title Use .rc files in TSM instead of powershell scripts to generate the default strings needed Use .rc files in TSM instead of string literals Mar 11, 2024
@DHowett
Copy link
Member

DHowett commented Mar 11, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DHowett DHowett requested a review from lhecker March 12, 2024 14:30
@DHowett DHowett merged commit 566b660 into main Mar 14, 2024
20 checks passed
@DHowett DHowett deleted the dev/pabhoj/defaults_resource branch March 14, 2024 20:50
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