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

Fix two sources of runtime exceptions #18628

Merged
merged 2 commits into from
Feb 25, 2025
Merged

Fix two sources of runtime exceptions #18628

merged 2 commits into from
Feb 25, 2025

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Feb 25, 2025

  • _ApplyLanguageSettingChange calls PrimaryLanguageOverride
    (the WinRT API function) and we would call it every time a new
    window is created. Now it's only called on settings load.
  • _RegisterTabEvents would listen for "Content" changes which can
    be null. IVector::Append throws if a null object is given.
    In our case, it's null if the content got erased with nothing.

Additionally, this fixes a bug where we wouldn't call
_ProcessLazySettingsChanges on startup. This is important if the
settings file was changed while Windows Terminal wasn't running.

Lastly, there's a lifetime fix in this PR, which is a one-line change
and I didn't want to make a separate PR for that.

@lhecker lhecker added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Priority-3 A description (P3) labels Feb 25, 2025
@lhecker lhecker merged commit ff9664d into main Feb 25, 2025
19 checks passed
@lhecker lhecker deleted the dev/lhecker/exceptions branch February 25, 2025 19:50
DHowett pushed a commit that referenced this pull request Feb 26, 2025
* `_ApplyLanguageSettingChange` calls `PrimaryLanguageOverride`
  (the WinRT API function) and we would call it every time a new
  window is created. Now it's only called on settings load.
* `_RegisterTabEvents` would listen for "Content" changes which can
  be null. `IVector::Append` throws if a null object is given.
  In our case, it's null if the content got erased with nothing.

Additionally, this fixes a bug where we wouldn't call
`_ProcessLazySettingsChanges` on startup. This is important if the
settings file was changed while Windows Terminal wasn't running.

Lastly, there's a lifetime fix in this PR, which is a one-line change
and I didn't want to make a separate PR for that.

(cherry picked from commit ff9664d)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgXsQ60
Service-Version: 1.23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants