-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Show a double width cursor for double width characters #5319
Merged
Merged
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
a2dc867
first lets add back the code that crashes
leonMSFT 56392f6
adding a test that may or may not work
leonMSFT b86d85e
test case
leonMSFT 9e149cb
i just add debug for now
leonMSFT 7574e9c
Merge branch 'master' into dev/lelian/bigcursor
leonMSFT 5fcbac7
mAYBE
leonMSFT 32af07a
ridding myself of comments
leonMSFT 69b0f27
y fail if it wouldn't matter hmm
leonMSFT 21fd1b3
why don't we just take these calls and place them over here
leonMSFT 8b46679
Merge branch 'master' into dev/lelian/bigcursor
leonMSFT 91e4fb8
added test case to idk if this is right place
leonMSFT 51f6cce
remove this
leonMSFT 640bc69
reverting this file
leonMSFT ae430ce
wtf
leonMSFT 3509c9f
build is complaining, more returns
leonMSFT 8e70d83
what about nOW
leonMSFT 1a3a312
this is a much better thing
leonMSFT 4c7a47e
Merge branch 'dev/lelian/bigcursor' of github.com:microsoft/terminal …
leonMSFT 3e4252b
conhost can also use this, and i forgot to put something back in
leonMSFT cf32371
i am getting absolutely destroyed by mr static
leonMSFT 2216cb9
comment
leonMSFT File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In screenInfo, you said...
Should we just
CATCH_RETURN()
here then?Also, if we return on line 221, would we be fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I wanted to make sure
_buffer
needs to EndDefer no matter what before we get out of this function. If the try failed at all and entered the catch, I need to callEndDeferDrawing()
before we return the exception. ScreenInfo seemed to be slightly different in that there wasn't a try-catch wrapping the Reflow call.I actually did not notice the
RETURN_IF_FAILED
for Reflow... Does that just totally skip the catch block? If so, I need to figure out where to EndDefer for_buffer
🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a glimpse at
RETURN_IF_FAILED
, yup. We'd return so the catch block never gets executed.You could probably get around it with a
wil::scope_exit
though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I basically just separated out the
RETURN_IF_FAILED
call into its innards, but slapped anEndDefer
call before it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd somewhat prefer a scope_exit if possible. I'm happy to merge this as-is if a scoped end-defer isn't easy, but i'm worried that we have three different "end defer" locations here, and they refer to multiple different text buffers. idk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh crap I'm so sorry @carlos-zamora 😢 , I didn't refresh my tab this morning when I replied to this thread so I didn't see the
scope_exit
comment before pushing my wacky change! Both of you are totally right, it's a much better approach 😅