-
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
Make selection an exclusive range #18106
base: main
Are you sure you want to change the base?
Changes from all commits
aa95790
4697f49
4364643
014f338
ba673a4
cd2da0e
ff45eb9
659cd02
e7ca807
7ff4838
928e9a9
9d361d2
845e43d
8b996a9
df89ad0
9e9db42
eea8399
7bfd647
52f3bc0
408d149
57b82cb
e98033e
e621c1c
83d2bff
6aed5f9
7a8d448
6b99816
d179e9c
d2ed09e
37ad74f
dbfdf81
bee9371
44a1e42
4764653
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -411,16 +411,13 @@ Microsoft::Console::ICU::unique_uregex Microsoft::Console::ICU::CreateRegex(cons | |
return unique_uregex{ re }; | ||
} | ||
|
||
// Returns an inclusive point range given a text start and end position. | ||
// Returns a half-open [beg,end) range given a text start and end position. | ||
// This function is designed to be used with uregex_start64/uregex_end64. | ||
til::point_span Microsoft::Console::ICU::BufferRangeFromMatch(UText* ut, URegularExpression* re) | ||
{ | ||
UErrorCode status = U_ZERO_ERROR; | ||
const auto nativeIndexBeg = uregex_start64(re, 0, &status); | ||
auto nativeIndexEnd = uregex_end64(re, 0, &status); | ||
|
||
// The parameters are given as a half-open [beg,end) range, but the point_span we return in closed [beg,end]. | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
nativeIndexEnd--; | ||
const auto nativeIndexEnd = uregex_end64(re, 0, &status); | ||
|
||
const auto& textBuffer = *static_cast<const TextBuffer*>(ut->context); | ||
til::point_span ret; | ||
|
@@ -439,7 +436,7 @@ til::point_span Microsoft::Console::ICU::BufferRangeFromMatch(UText* ut, URegula | |
if (utextAccess(ut, nativeIndexEnd, true)) | ||
{ | ||
const auto y = accessCurrentRow(ut); | ||
ret.end.x = textBuffer.GetRowByOffset(y).GetTrailingColumnAtCharOffset(ut->chunkOffset); | ||
ret.end.x = textBuffer.GetRowByOffset(y).GetLeadingColumnAtCharOffset(ut->chunkOffset); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lhecker ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The offsets are exclusive now. This means the chunkOffset now refers to the first character after the matched range. In turn, this means that if there's a wide glyph after the matched range, we want its leading half, because the exclusive end should be exactly 1 past the end (and not 2). |
||
ret.end.y = y; | ||
} | ||
else | ||
|
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.
what's all this?
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.
These changes are necessary so that both accessor functions work reliably with an exclusive target char offset. I suggested to rename the member to make its usage a bit easier to understand, now that it refers to the length and not length-1 anymore. I did this in collaboration with Carlos. 🙂