-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add buttons for selecting commands, output to context menu #15020
Merged
microsoft-github-policy-service
merged 72 commits into
main
from
dev/migrie/f/rclick-select-command
Apr 25, 2023
Merged
Changes from all commits
Commits
Show all changes
72 commits
Select commit
Hold shift + click to select a range
eaf1a92
okay maybe I have silly free time ideas too
zadjii-msft 7001e93
forwarded_event, string tests
zadjii-msft eb88cd3
practical use, look
zadjii-msft b535a5f
I suppose this is what happens when I have four uninterrupted hours a…
zadjii-msft 520c89f
sad beeps
zadjii-msft eb7d2be
I am incapable of not making 100 tiny commits, this was already way t…
zadjii-msft e6b33c3
yea that's everything
zadjii-msft 29d454b
90% plumbing, 10% bad implementation of making sure we right-clicked …
zadjii-msft 451d8f3
move this code to interactivity. Scaling of event is wrong now
zadjii-msft 99b84a6
Merge remote-tracking branch 'origin/main' into dev/migrie/f/3337-jus…
zadjii-msft dc2ebaf
use resource strings too. Last commit of stuff from before paternity …
zadjii-msft 61a9c80
Migrate spelling-0.0.21 changes from main
DHowett 5398ea3
mostly, the plumbing for this feature
zadjii-msft 23f9527
this actually just selects the command+1 character
zadjii-msft dbb4db3
this gets the selection region actually correct
zadjii-msft d867a61
Add support for going up and down
zadjii-msft e76564e
this makes it _less_ painful
zadjii-msft ce6a2e7
you-donkey.png
zadjii-msft 191bca8
Merge branch 'main' into dev/migrie/f/3337-just-for-funsies
zadjii-msft 6a60053
a commandbarflyout intsead, with a slightly more idomatic implementation
zadjii-msft 2dd01fb
open at the mouse
zadjii-msft 2ee4111
adapt the previous action adder for the new menu
zadjii-msft 854335b
cleanup; don't duplicate selection entries each time; open in the rig…
zadjii-msft 0328b4f
icons, and dismiss the menu when the action's done
zadjii-msft 08ae0ec
cleanup for review
zadjii-msft d6fe520
'cleanup
zadjii-msft 7aef3af
bommand
zadjii-msft c77f959
PR nits
zadjii-msft ab55a8d
Revert "sad beeps"
zadjii-msft 559ecdb
Revert "I suppose this is what happens when I have four uninterrupted…
zadjii-msft 9ff0abe
Revert "practical use, look"
zadjii-msft 564102b
Revert "forwarded_event, string tests"
zadjii-msft a1d5329
Revert "okay maybe I have silly free time ideas too"
zadjii-msft 0e546a6
Merge remote-tracking branch 'origin/main' into dev/migrie/f/4588-sel…
zadjii-msft 35b519a
use resorces
zadjii-msft 2a6520d
cleanup
zadjii-msft 77e4289
oh boy missed a pair
zadjii-msft d01cab3
Merge branch 'main' into dev/migrie/f/3337-just-for-funsies
zadjii-msft c4f3011
Merge branch 'main' into dev/migrie/f/4588-select-shell-output
zadjii-msft 654baa7
most of the PR feedback
zadjii-msft 23b46cb
Merge branch 'dev/migrie/f/3337-just-for-funsies' into dev/migrie/f/r…
zadjii-msft 99e556b
Merge branch 'dev/migrie/f/4588-select-shell-output' into dev/migrie/…
zadjii-msft dde4e9e
This is a start. Selecting the output is almost always wrong and I do…
zadjii-msft 449954e
This is better
zadjii-msft 5a4f802
only show the menu when you've actually got shell integration enabled
zadjii-msft 50d4683
I just love it so much
zadjii-msft eaae3ce
Merge branch 'main' into dev/migrie/f/4588-select-shell-output
zadjii-msft b2df084
Merge branch 'dev/migrie/f/4588-select-shell-output' of https://githu…
zadjii-msft e418751
Merge branch 'dev/migrie/f/4588-select-shell-output' into dev/migrie/…
zadjii-msft de3dada
Merge remote-tracking branch 'origin/main' into dev/migrie/f/4588-sel…
zadjii-msft 7d17e6e
Merge branch 'dev/migrie/f/4588-select-shell-output' into dev/migrie/…
zadjii-msft 52c4439
Merge remote-tracking branch 'origin/main' into dev/migrie/f/4588-sel…
zadjii-msft fe3ecb9
start working on a test
zadjii-msft 9ec3d19
Merge branch 'dev/migrie/f/4588-select-shell-output' into dev/migrie/…
zadjii-msft 275f3de
Fix the test to work as expected
zadjii-msft bd2e0b8
Fix the test to work as expected
zadjii-msft 6ab1b6f
more tests
zadjii-msft cb03fb2
Merge branch 'dev/migrie/f/4588-select-shell-output' into dev/migrie/…
zadjii-msft 3f91664
I suppose we don't need to reverse-iterate
zadjii-msft 0be83bc
Merge remote-tracking branch 'origin/main' into dev/migrie/f/4588-sel…
zadjii-msft 9016865
Merge branch 'dev/migrie/f/4588-select-shell-output' into dev/migrie/…
zadjii-msft fe2d54e
Merge remote-tracking branch 'origin/main' into dev/migrie/f/4588-sel…
zadjii-msft d5af16c
this was leonard's suggestion
zadjii-msft 7c709cf
this is leonard's thing, for command output too
zadjii-msft e244b5e
Merge branch 'dev/migrie/f/4588-select-shell-output' into dev/migrie/…
zadjii-msft 6cda6aa
Merge branch 'main' into dev/migrie/f/rclick-select-command
zadjii-msft 242b318
Merge branch 'dev/migrie/f/rclick-select-command' of https://github.c…
zadjii-msft 278e77f
Somehow, this returned
zadjii-msft 7a95b13
start de-duping code
zadjii-msft 30247c9
more deduplication for fun and profit
zadjii-msft d5347d0
Really trying my best to get rid of code duplication here
zadjii-msft de46ed8
this was apparently noexcept
zadjii-msft 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2182,10 +2182,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
} | ||
} | ||
|
||
void ControlCore::_selectSpan(til::point_span s) | ||
{ | ||
const auto bufferSize{ _terminal->GetTextBuffer().GetSize() }; | ||
bufferSize.DecrementInBounds(s.end); | ||
|
||
auto lock = _terminal->LockForWriting(); | ||
_terminal->SelectNewRegion(s.start, s.end); | ||
_renderer->TriggerSelection(); | ||
} | ||
|
||
void ControlCore::SelectCommand(const bool goUp) | ||
{ | ||
const til::point start = HasSelection() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : | ||
_terminal->GetTextBuffer().GetCursor().GetPosition(); | ||
|
||
std::optional<DispatchTypes::ScrollMark> nearest{ std::nullopt }; | ||
const auto& marks{ _terminal->GetScrollMarks() }; | ||
|
||
|
@@ -2217,20 +2228,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
{ | ||
const auto start = nearest->end; | ||
auto end = *nearest->commandEnd; | ||
|
||
const auto bufferSize{ _terminal->GetTextBuffer().GetSize() }; | ||
bufferSize.DecrementInBounds(end); | ||
|
||
auto lock = _terminal->LockForWriting(); | ||
_terminal->SelectNewRegion(start, end); | ||
_renderer->TriggerSelection(); | ||
_selectSpan(til::point_span{ start, end }); | ||
} | ||
} | ||
|
||
void ControlCore::SelectOutput(const bool goUp) | ||
{ | ||
const til::point start = HasSelection() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : | ||
_terminal->GetTextBuffer().GetCursor().GetPosition(); | ||
|
||
std::optional<DispatchTypes::ScrollMark> nearest{ std::nullopt }; | ||
const auto& marks{ _terminal->GetScrollMarks() }; | ||
|
||
|
@@ -2256,13 +2262,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
{ | ||
const auto start = *nearest->commandEnd; | ||
auto end = *nearest->outputEnd; | ||
|
||
const auto bufferSize{ _terminal->GetTextBuffer().GetSize() }; | ||
bufferSize.DecrementInBounds(end); | ||
|
||
auto lock = _terminal->LockForWriting(); | ||
_terminal->SelectNewRegion(start, end); | ||
_renderer->TriggerSelection(); | ||
_selectSpan(til::point_span{ start, end }); | ||
} | ||
} | ||
|
||
|
@@ -2301,4 +2301,110 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
} | ||
} | ||
} | ||
|
||
void ControlCore::AnchorContextMenu(const til::point viewportRelativeCharacterPosition) | ||
{ | ||
// viewportRelativeCharacterPosition is relative to the current | ||
// viewport, so adjust for that: | ||
_contextMenuBufferPosition = _terminal->GetViewport().Origin() + viewportRelativeCharacterPosition; | ||
} | ||
|
||
void ControlCore::_contextMenuSelectMark( | ||
const til::point& pos, | ||
const std::function<bool(const DispatchTypes::ScrollMark&)>& filter, | ||
const std::function<til::point_span(const DispatchTypes::ScrollMark&)>& getSpan) | ||
{ | ||
// Do nothing if the caller didn't give us a way to get the span to select for this mark. | ||
if (!getSpan) | ||
{ | ||
return; | ||
} | ||
Comment on lines
+2317
to
+2321
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. You can just write |
||
const auto& marks{ _terminal->GetScrollMarks() }; | ||
for (auto&& m : marks) | ||
zadjii-msft marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
// If the caller gave us a way to filter marks, check that now. | ||
// This can be used to filter to only marks that have a command, or output. | ||
if (filter && filter(m)) | ||
{ | ||
continue; | ||
} | ||
// If they clicked _anywhere_ in the mark... | ||
const auto [markStart, markEnd] = m.GetExtent(); | ||
if (markStart <= pos && | ||
markEnd >= pos) | ||
{ | ||
// ... select the part of the mark the caller told us about. | ||
_selectSpan(getSpan(m)); | ||
// And quick bail | ||
return; | ||
} | ||
} | ||
} | ||
|
||
void ControlCore::ContextMenuSelectCommand() | ||
{ | ||
_contextMenuSelectMark( | ||
_contextMenuBufferPosition, | ||
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasCommand(); }, | ||
[](const DispatchTypes::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; }); | ||
} | ||
void ControlCore::ContextMenuSelectOutput() | ||
{ | ||
_contextMenuSelectMark( | ||
_contextMenuBufferPosition, | ||
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasOutput(); }, | ||
[](const DispatchTypes::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; }); | ||
} | ||
|
||
bool ControlCore::_clickedOnMark( | ||
const til::point& pos, | ||
const std::function<bool(const DispatchTypes::ScrollMark&)>& filter) | ||
{ | ||
// Don't show this if the click was on the selection | ||
if (_terminal->IsSelectionActive() && | ||
_terminal->GetSelectionAnchor() <= pos && | ||
_terminal->GetSelectionEnd() >= pos) | ||
{ | ||
return false; | ||
} | ||
|
||
// DO show this if the click was on a mark with a command | ||
const auto& marks{ _terminal->GetScrollMarks() }; | ||
for (auto&& m : marks) | ||
zadjii-msft marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
if (filter && filter(m)) | ||
{ | ||
continue; | ||
} | ||
const auto [start, end] = m.GetExtent(); | ||
if (start <= pos && | ||
end >= pos) | ||
{ | ||
return true; | ||
} | ||
} | ||
|
||
// Didn't click on a mark with a command - don't show. | ||
return false; | ||
} | ||
|
||
// Method Description: | ||
// * Don't show this if the click was on the _current_ selection | ||
// * Don't show this if the click wasn't on a mark with at least a command | ||
// * Otherwise yea, show it. | ||
bool ControlCore::ShouldShowSelectCommand() | ||
{ | ||
// Relies on the anchor set in AnchorContextMenu | ||
return _clickedOnMark(_contextMenuBufferPosition, | ||
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasCommand(); }); | ||
} | ||
|
||
// Method Description: | ||
// * Same as ShouldShowSelectCommand, but with the mark needing output | ||
bool ControlCore::ShouldShowSelectOutput() | ||
{ | ||
// Relies on the anchor set in AnchorContextMenu | ||
return _clickedOnMark(_contextMenuBufferPosition, | ||
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasOutput(); }); | ||
} | ||
} |
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
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
Oops, something went wrong.
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.
This can also be achieved using regular function pointers like so:
The remaining code should still compile the same. This makes it easier on the compiler (
std::function
is a very large, very very complex template, has lots of exception handling routines and is 64 bytes large) and easier on the debugger, which will have a very easy time tracing raw function pointers.If you'd like to I'd be happy to help you with such a change.
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'm assuming these comments went unaddressed
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.
whoops sorry, fixed in #15233