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

Fixed throwing floating point cast #3784

Merged
merged 1 commit into from
Dec 2, 2019

Conversation

mkitzan
Copy link
Contributor

@mkitzan mkitzan commented Nov 30, 2019

Summary of the Pull Request

Replaced a gsl::narrow call to gsl::narrow_cast call. The gsl::narrow call used to throw when the user had custom display scaling due to a bad comparison between floating point values.

References

Possible other startup crashes. I'll update this as they're found.

PR Checklist

Detailed Description of the Pull Request / Additional comments

It's a one line fix. If you want more context, here's the full description of the problem.

Validation Steps Performed

Set my machine to a custom scaling and opened a fixed build of the WT. My WT started up without crashing and continued to operate without issues (including maximizing, minimizing, and fullscreen toggle).

@mkitzan mkitzan marked this pull request as ready for review November 30, 2019 05:25
@DHowett-MSFT DHowett-MSFT added the Needs-Second It's a PR that needs another sign-off label Nov 30, 2019
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea this looks fine to me. Good catch!

@zadjii-msft zadjii-msft merged commit b17038b into microsoft:master Dec 2, 2019
@zadjii-msft zadjii-msft added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. labels Dec 2, 2019
@mkitzan mkitzan deleted the floating-point-comp branch December 2, 2019 17:42
DHowett-MSFT pushed a commit that referenced this pull request Dec 2, 2019
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Replaced a `gsl::narrow` call to `gsl::narrow_cast` call. The `gsl::narrow` call used to throw when the user had custom display scaling due to a bad comparison between floating point values.

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
Possible other [startup crashes](#3749 (comment)). I'll update this as they're found.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #3749, likely #3747
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3749

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
It's a one line fix. If you want more context, here's the [full description](#3749 (comment)) of the problem.

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Set my machine to a custom scaling  and opened a fixed build of the WT. My WT started up without crashing and continued to operate without issues (including maximizing, minimizing, and fullscreen toggle).

(cherry picked from commit b17038b)
@ghost
Copy link

ghost commented Dec 6, 2019

🎉Windows Terminal Preview v0.7.3382.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crashes when a custom scaling size is set
3 participants