Skip to content

Commit

Permalink
Implement EnableColorSelection
Browse files Browse the repository at this point in the history
As described in microsoft#9583, this change implements the legacy conhost "EnableColorSelection" feature.

@zadjii-msft was super nice and provided the outline/plumbing (WinRT classes and such) as a hackathon-type project (thank you!)--a "SelectionColor" runtimeclass, a ColorSelection method on the ControlCore runtimeclass, associated plumbing through the layers; plus the action-and-args plumbing to allow hooking up a basic "ColorSelection" action, which allows you to put actions in your settings JSON like so:

```json
{
    "command":
    {
        "action": "experimental.colorSelection",
        "foreground": "#0f3"
    },
    "keys": "alt+4"
},
```

On top of that foundation, I added a couple of things:
* The ability to specify indexes for colors, in addition to RGB and RRGGBB colors.
  - It's a bit hacky, because there are some conversions that fight against sneaking an "I'm an index" flag in the alpha channel.
* A new "matchMode" parameter on the action, allowing you to say if you want to only color the current selection ("0") or all matches ("1").
  - I made it an int, because I'd like to enable at least one other "match mode" later, but it requires me/someone to fix up search.cpp to handle regex first.
  - Search used an old UIA "ColorSelection" method which was previously `E_NOTIMPL`, but is now wired up. Don't know what/if anything else uses this.
* An uber-toggle setting, "EnableColorSelection", which allows you to set a single `bool` in your settings JSON, to light up all the keybindings you would expect from the legacy "EnableColorSelection" feature:
    - alt+[0..9]: color foreground
    - alt+shift+[0..9]: color foreground, all matches
    - ctrl+[0..9]: color background
    - ctrl+shift+[0..9]: color background, all matches
 * A few of the actions cannot be properly invoked via their keybindings, due to microsoft#13124. `*!*` But they work if you do them from the command palette.
  * If you have "`EnableColorSelection : true`" in your settings JSON, but then specify a different action in your JSON that uses the same key binding as a color selection keybinding, your custom one wins, which I think is the right thing.
* I fixed what I think was a bug in search.cpp, which also affected the legacy EnableColorSelection feature: due to a non-inclusive coordinate comparison, you were not allowed to color a single character; but I see no reason why that should be disallowed. Now you can make all your `X`s red if you like.

"Soft" spots:
* I was a bit surprised at some of the helpers I had to provide in textBuffer.cpp. Perhaps there are existing methods that I didn't find?
  • Loading branch information
jazzdelightsme committed Aug 4, 2022
1 parent 74cdffe commit b5a59c0
Show file tree
Hide file tree
Showing 29 changed files with 1,185 additions and 98 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ istream
IStringable
ITab
ITaskbar
itow
IUri
IVirtual
KEYSELECT
Expand Down
42 changes: 42 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@
"scrollToMark",
"clearMark",
"clearAllMarks",
"experimental.colorSelection",
"unbound"
],
"type": "string"
Expand Down Expand Up @@ -836,6 +837,39 @@
}
]
},
"ColorSelectionAction": {
"description": "Arguments corresponding to a Color Selection Action",
"allOf": [
{
"$ref": "#/$defs/ShortcutAction"
},
{
"properties": {
"action": {
"type": "string",
"const": "experimental.colorSelection"
},
"matchMode": {
"default": 0,
"description": "Specifies if only the selected text should be colored (0), or all instances of selected text (case-insensitive) (1).",
"maximum": 1,
"minimum": 0,
"type": "integer"
},
"foreground": {
"type": "string",
"default": "",
"description": "The foreground color to use (either \"#RGB\" or \"#RRGGBB\" for an exact color, or \"iNN\" for an indexed color)."
},
"background": {
"type": "string",
"default": "",
"description": "The background color to use (either \"#RGB\" or \"#RRGGBB\" for an exact color, or \"iNN\" for an indexed color)."
}
}
}
]
},
"OpenSettingsAction": {
"description": "Arguments corresponding to a Open Settings Action",
"allOf": [
Expand Down Expand Up @@ -1611,6 +1645,9 @@
{
"$ref": "#/$defs/AdjustOpacityAction"
},
{
"$ref": "#/$defs/ColorSelectionAction"
},
{
"type": "null"
}
Expand Down Expand Up @@ -1733,6 +1770,11 @@
"description": "When set to true, URLs will be detected by the Terminal. This will cause URLs to underline on hover and be clickable by pressing Ctrl.",
"type": "boolean"
},
"experimental.enableColorSelection": {
"default": false,
"description": "When set to true, adds 40 preset \"Color Selection\" actions (keybindings) to allow colorizing selected text via keystroke, similar to the legacy conhost EnableColorSelection feature (such as alt+6 to color the selection red).",
"type": "boolean"
},
"disableAnimations": {
"default": false,
"description": "When set to `true`, visual animations will be disabled across the application.",
Expand Down
7 changes: 7 additions & 0 deletions src/buffer/out/LineRendition.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ constexpr til::inclusive_rect ScreenToBufferLine(const til::inclusive_rect& line
return { line.Left >> scale, line.Top, line.Right >> scale, line.Bottom };
}

constexpr til::point ScreenToBufferLine(const til::point& line, const LineRendition lineRendition)
{
// Use shift right to quickly divide the Left and Right by 2 for double width lines.
const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1;
return { line.X >> scale, line.Y };
}

constexpr til::inclusive_rect BufferToScreenLine(const til::inclusive_rect& line, const LineRendition lineRendition)
{
// Use shift left to quickly multiply the Left and Right by 2 for double width lines.
Expand Down
13 changes: 5 additions & 8 deletions src/buffer/out/search.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,14 @@ void Search::Select() const
}

// Routine Description:
// - In console host, we take the found word and apply the given color to it in the screen buffer
// - In Windows Terminal, we just select the found word, but we do not modify the buffer
// - Applies the supplied TextAttribute to the current search result.
// Arguments:
// - ulAttr - The legacy color attribute to apply to the word
// - attr - The attribute to apply to the result
void Search::Color(const TextAttribute attr) const
{
// Only select if we've found something.
if (_coordSelStart != _coordSelEnd)
{
_uiaData.ColorSelection(_coordSelStart, _coordSelEnd, attr);
}
// Note that _coordSelStart may be equal to _coordSelEnd (but it's an inclusive
// selection: if they are equal, it means we are applying to a single character).
_uiaData.ColorSelection(_coordSelStart, _coordSelEnd, attr);
}

// Routine Description:
Expand Down
139 changes: 138 additions & 1 deletion src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1611,7 +1611,7 @@ bool TextBuffer::MoveToPreviousGlyph(til::point& pos, std::optional<til::point>
// - bufferCoordinates: when enabled, treat the coordinates as relative to
// the buffer rather than the screen.
// Return Value:
// - the delimiter class for the given char
// - One or more rects corresponding to the selection area
const std::vector<til::inclusive_rect> TextBuffer::GetTextRects(til::point start, til::point end, bool blockSelection, bool bufferCoordinates) const
{
std::vector<til::inclusive_rect> textRects;
Expand Down Expand Up @@ -1660,6 +1660,72 @@ const std::vector<til::inclusive_rect> TextBuffer::GetTextRects(til::point start
return textRects;
}

// Method Description:
// - Computes the span(s) for the given selection
// - If not a blockSelection, returns a single span (start - end)
// - Else if a blockSelection, returns spans corresponding to each line in the block selection
// Arguments:
// - start: beginning of the text region of interest (inclusive)
// - end: the other end of the text region of interest (inclusive)
// - blockSelection: when enabled, get spans for each line covered by the block
// - bufferCoordinates: when enabled, treat the coordinates as relative to
// the buffer rather than the screen.
// Return Value:
// - one or more sets of start-end coordinates, representing spans of text in the buffer
const std::vector<std::tuple<til::point, til::point>> TextBuffer::GetTextSpans(til::point start, til::point end, bool blockSelection, bool bufferCoordinates) const
{
std::vector<std::tuple<til::point, til::point>> textSpans;

if (blockSelection)
{
// If blockSelection, this is effectively the same operation as GetTextRects, but
// expressed in til::point coordinates.
auto rects = GetTextRects(start, end, /*blockSelection*/ true, bufferCoordinates);
textSpans.reserve(rects.size());

for (auto rect : rects)
{
til::point first = { rect.Left, rect.Top };
til::point second = { rect.Right, rect.Bottom };
auto span = std::make_tuple(first, second);
textSpans.emplace_back(span);
}
}
else
{
const auto bufferSize = GetSize();

// (0,0) is the top-left of the screen
// the physically "higher" coordinate is closer to the top-left
// the physically "lower" coordinate is closer to the bottom-right
auto [higherCoord, lowerCoord] = start <= end ?
std::make_tuple(start, end) :
std::make_tuple(end, start);

textSpans.reserve(1);

// If we were passed screen coordinates, convert the given range into
// equivalent buffer offsets, taking line rendition into account.
if (!bufferCoordinates)
{
higherCoord = ScreenToBufferLine(higherCoord, GetLineRendition(higherCoord.Y));
lowerCoord = ScreenToBufferLine(lowerCoord, GetLineRendition(lowerCoord.Y));
}

til::inclusive_rect asRect = { higherCoord.X, higherCoord.Y, lowerCoord.X, lowerCoord.Y };
_ExpandTextRow(asRect);
higherCoord.X = asRect.Left;
higherCoord.Y = asRect.Top;
lowerCoord.X = asRect.Right;
lowerCoord.Y = asRect.Bottom;

auto span = std::make_tuple(higherCoord, lowerCoord);
textSpans.emplace_back(span);
}

return textSpans;
}

// Method Description:
// - Expand the selection row according to include wide glyphs fully
// - this is particularly useful for box selections (ALT + selection)
Expand Down Expand Up @@ -1832,6 +1898,77 @@ const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF,
return data;
}

size_t TextBuffer::SpanLength(const til::point coordStart, const til::point coordEnd) const
{
assert((coordEnd.Y > coordStart.Y) ||
((coordEnd.Y == coordStart.Y) && (coordEnd.X >= coordStart.X)));

// Note that this could also be computed using CompareInBounds, but that function
// seems disfavored lately.
//
// CompareInBounds version:
//
// const auto bufferSize = GetSize();
// // Note that we negate because CompareInBounds is backwards from what we are trying to calculate.
// auto length = - bufferSize.CompareInBounds(coordStart, coordEnd);
// length += 1; // because we need "inclusive" behavior.

const auto rowSize = gsl::narrow<SHORT>(GetRowByOffset(0).size());

size_t length = (static_cast<size_t>(coordEnd.Y) - coordStart.Y) * rowSize;
length += (static_cast<size_t>(coordEnd.X) - coordStart.X) + 1; // "+1" is because we need "inclusive" behavior

return length;
}

// Routine Description:
// - Retrieves the plain text data between the specified coordinates.
// Arguments:
// - trimTrailingWhitespace - remove the trailing whitespace at the end of the result.
// - start - where to start getting text (should be at or prior to "end")
// - end - where to end getting text
// Return Value:
// - Just the text.
const std::wstring TextBuffer::GetPlainText(const bool trimTrailingWhitespace,
const til::point& start,
const til::point& end) const
{
std::wstring text;
// TODO: should I put in protections for start coming before end?
auto spanLength = SpanLength(start, end);
text.reserve(spanLength);

auto it = GetCellDataAt(start);

// copy char data into the string buffer, skipping trailing bytes
// TODO: is using spanLength like this the right way to do it?
while (it && ((spanLength) > 0))
{
const auto& cell = *it;
spanLength--;

if (!cell.DbcsAttr().IsTrailing())
{
const auto chars = cell.Chars();
text.append(chars);
}
#pragma warning(suppress : 26444)
// TODO GH 2675: figure out why there's custom construction/destruction happening here
it++;
}

if (trimTrailingWhitespace)
{
// remove the spaces at the end (aka trim the trailing whitespace)
while (!text.empty() && text.back() == UNICODE_SPACE)
{
text.pop_back();
}
}

return text;
}

// Routine Description:
// - Generates a CF_HTML compliant structure based on the passed in text and color data
// Arguments:
Expand Down
7 changes: 7 additions & 0 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ class TextBuffer final
bool MoveToPreviousGlyph(til::point& pos, std::optional<til::point> limitOptional = std::nullopt) const;

const std::vector<til::inclusive_rect> GetTextRects(til::point start, til::point end, bool blockSelection, bool bufferCoordinates) const;
const std::vector<std::tuple<til::point, til::point>> GetTextSpans(til::point start, til::point end, bool blockSelection, bool bufferCoordinates) const;

void AddHyperlinkToMap(std::wstring_view uri, uint16_t id);
std::wstring GetHyperlinkUriFromId(uint16_t id) const;
Expand All @@ -182,12 +183,18 @@ class TextBuffer final
std::vector<std::vector<COLORREF>> BkAttr;
};

size_t SpanLength(const til::point coordStart, const til::point coordEnd) const;

const TextAndColor GetText(const bool includeCRLF,
const bool trimTrailingWhitespace,
const std::vector<til::inclusive_rect>& textRects,
std::function<std::pair<COLORREF, COLORREF>(const TextAttribute&)> GetAttributeColors = nullptr,
const bool formatWrappedRows = false) const;

const std::wstring GetPlainText(const bool trimTrailingWhitespace,
const til::point& start,
const til::point& end) const;

static std::string GenHTML(const TextAndColor& rows,
const int fontHeightPoints,
const std::wstring_view fontFaceName,
Expand Down
15 changes: 15 additions & 0 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1122,4 +1122,19 @@ namespace winrt::TerminalApp::implementation
args.Handled(handled);
}
}

void TerminalPage::_HandleColorSelection(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (args)
{
if (const auto& realArgs = args.ActionArgs().try_as<ColorSelectionArgs>())
{
const auto res = _ApplyToActiveControls([&](auto& control) {
control.ColorSelection(realArgs.Foreground(), realArgs.Background(), realArgs.MatchMode());
});
args.Handled(res);
}
}
}
}
37 changes: 37 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "../../renderer/atlas/AtlasEngine.h"
#include "../../renderer/dx/DxRenderer.hpp"

#include "SelectionColor.g.cpp"
#include "ControlCore.g.cpp"

using namespace ::Microsoft::Console::Types;
Expand Down Expand Up @@ -2136,4 +2137,40 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
}
}

void ControlCore::ColorSelection(Control::SelectionColor fg, Control::SelectionColor bg, uint32_t matchMode)
{
if (HasSelection())
{
const auto pForeground = winrt::get_self<implementation::SelectionColor>(fg);
const auto pBackground = winrt::get_self<implementation::SelectionColor>(bg);

TextColor foregroundAsTextColor;
TextColor backgroundAsTextColor;

if (pForeground)
{
foregroundAsTextColor = pForeground->Color();
}

if (pBackground)
{
backgroundAsTextColor = pBackground->Color();
}

TextAttribute attr;
attr.SetForeground(foregroundAsTextColor);
attr.SetBackground(backgroundAsTextColor);

_terminal->ColorSelection(attr, matchMode);
_terminal->ClearSelection();
if (matchMode > 0)
{
// ClearSelection will invalidate the selection area... but if we are
// coloring other matches, then we need to make sure those get redrawn,
// too.
_renderer->TriggerRedrawAll();
}
}
}
}
Loading

0 comments on commit b5a59c0

Please sign in to comment.