Polish MarkMode interactions with ExpandSelectionToWord and existing selection #13893
Conversation
| _selection->end = buffer.GetWordEnd(_selection->end, _wordDelimiters); | ||
|
|
||
| // if we're targetting both endpoints, instead just target "end" | ||
| if (WI_IsFlagSet(_selectionEndpoint, SelectionEndpoint::Start) && WI_IsFlagSet(_selectionEndpoint, SelectionEndpoint::End)) |
There was a problem hiding this comment.
I tried 😭. Compiler gave me sass about & not being defined or something like that even though this is at the top of the file:
DEFINE_ENUM_FLAG_OPERATORS(Terminal::SelectionEndpoint);
There was a problem hiding this comment.
Huh... that's weird. We do use WI_AreAllFlagsSet in TermControl.cpp already.
There was a problem hiding this comment.
with this specific enum type? If so, yes, thisd should work. If not, we probably need the DECLARE_ENUM_BOOLEAN_VALUES thing...
There was a problem hiding this comment.
Yeah :(
I'm curious. Imma try that declare enum bool thing.
There was a problem hiding this comment.
ugh, yeah I can't find that macro. We'll figure it out later.
There was a problem hiding this comment.
It's maddening that WI_AreAllFlagsClear works perfectly well nary 20 lines above.
There was a problem hiding this comment.
This is likely because of ADL failing on our nested type ;P
carlos-zamora
force-pushed
the
dev/cazamor/sln-polish
branch
from
5674123 to
85b219a
Compare
|
@lhecker added a fix for...
Shouldn't be controversial but feel free to disagree haha |
FYI: Using individual commits makes it somewhat difficult because they require manual cherry picking and can't go through the script. No need to change right now, but just for the future :) |
|
|
| else | ||
| { | ||
| const auto selectedText = _terminal->GetTextBuffer().GetPlainText(_terminal->GetSelectionAnchor(), _terminal->GetSelectionEnd()); | ||
| _OpenHyperlinkHandlers(*this, winrt::make<OpenHyperlinkEventArgs>(winrt::hstring{ selectedText })); |
There was a problem hiding this comment.
isn't this just gonna cause a crash later when it turns out that the thing isn't a hyperlink?
There was a problem hiding this comment.
No, we have logic somewhere such that a popup appears saying that it's not a hyperlink (I'll pull up a pic and attach it here later).
I'm pretty sure it's localized too. I'm just using what we have hahaha.
| @@ -517,11 +533,6 @@ void Terminal::SelectHyperlink(const SearchDirection dir) | |||
| _ScrollToPoint(_selection->end); | |||
| } | |||
|
|
|||
| bool Terminal::SelectionIsTargetingUrl() const noexcept | |||
| { | |||
| return _selectionIsTargetingUrl; | |||
There was a problem hiding this comment.
so _selectionIsTargetingUrl is now only used to know whether we are in URL jump mode?
There was a problem hiding this comment.
Ah, it's only so that we don't jump to the link we just jumped to when moving forward?
There was a problem hiding this comment.
Yup. It's so that we skip over the one that's currently selected.
DHowett
added
the
AutoMerge
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
|
🎉 Handy links: |
|
🎉 Handy links: |
References
#13854
NOTE: 3b53f3c can be serviced