Skip to content

Commit

Permalink
Implement EnableColorSelection (#13429)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

As described in #9583, this change implements the legacy conhost "EnableColorSelection" feature.

## Detailed Description of the Pull Request / Additional comments

@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 #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?
* Localization? Because there are so many (40!) actions, I went to some trouble to try to provide nice command/arg descriptions. But I don't know how localization works…


<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #9583
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed *(what would be the right place to add tests for this?)*
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated. *(is this needed?)*
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Validation Steps Performed
Just manual testing.
  • Loading branch information
jazzdelightsme authored Aug 31, 2022
1 parent a440e15 commit d11ea72
Show file tree
Hide file tree
Showing 35 changed files with 1,073 additions and 43 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
78 changes: 65 additions & 13 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
"type": "string",
"format": "color"
},
"ColorOrIndex": {
"default": "#",
"pattern": "^(?:#[A-Fa-f0-9]{3}(?:[A-Fa-f0-9]{3})?|i[A-Fa-f0-9]{2})$",
"type": "string",
"format": "color"
},
"Coordinates": {
"pattern": "^(-?\\d+)?(,\\s?(-?\\d+)?)?$",
"type": "string"
Expand Down Expand Up @@ -58,20 +64,20 @@
"default": "",
"description": "Sets the file location of the sound played when the application emits a BEL character. If the path is invalid no sound will be played. This property also accepts an array of sounds and the terminal will pick one at random.",
"oneOf": [
{
"type": [
"string",
"null"
]
},
{
"type": "array",
"items": {
"type": "string"
{
"type": [
"string",
"null"
]
},
{
"type": "array",
"items": {
"type": "string"
}
}
}
]
},
},
"AppearanceConfig": {
"properties": {
"colorScheme": {
Expand Down Expand Up @@ -378,6 +384,7 @@
"scrollToMark",
"clearMark",
"clearAllMarks",
"experimental.colorSelection",
"unbound"
],
"type": "string"
Expand Down Expand Up @@ -838,6 +845,43 @@
}
]
},
"ColorSelectionAction": {
"description": "Arguments corresponding to a Color Selection Action",
"allOf": [
{
"$ref": "#/$defs/ShortcutAction"
},
{
"properties": {
"action": {
"type": "string",
"const": "experimental.colorSelection"
},
"matchMode": {
"type": "string",
"default": "none",
"description": "Specifies if only the selected text should be colored (0), or all instances of selected text (case-insensitive) (1).",
"enum": [
"none",
"all"
]
},
"foreground": {
"$ref": "#/$defs/ColorOrIndex",
"type": "string",
"default": "",
"description": "The foreground color to use, as an RGB value (\"#rrggbb\"), or color index (\"iNN\"). If left unspecified it falls back to the default text foreground color."
},
"background": {
"$ref": "#/$defs/ColorOrIndex",
"type": "string",
"default": "",
"description": "The background color to use, as an RGB value (\"#rrggbb\"), or color index (\"iNN\"). If left unspecified it falls back to the default text background color."
}
}
}
]
},
"OpenSettingsAction": {
"description": "Arguments corresponding to a Open Settings Action",
"allOf": [
Expand Down Expand Up @@ -1613,6 +1657,9 @@
{
"$ref": "#/$defs/AdjustOpacityAction"
},
{
"$ref": "#/$defs/ColorSelectionAction"
},
{
"type": "null"
}
Expand Down Expand Up @@ -1735,6 +1782,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 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 Expand Up @@ -1974,7 +2026,7 @@
"afterLastTab",
"afterCurrentTab"
],
"type": "string"
"type": "string"
},
"autoHideWindow": {
"default": false,
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: 6 additions & 7 deletions src/buffer/out/search.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ using namespace Microsoft::Console::Types;
// - direction - The direction to search (upward or downward)
// - sensitivity - Whether or not you care about case
Search::Search(IUiaData& uiaData,
const std::wstring& str,
const std::wstring_view str,
const Direction direction,
const Sensitivity sensitivity) :
_direction(direction),
Expand All @@ -47,7 +47,7 @@ Search::Search(IUiaData& uiaData,
// - sensitivity - Whether or not you care about case
// - anchor - starting search location in screenInfo
Search::Search(IUiaData& uiaData,
const std::wstring& str,
const std::wstring_view str,
const Direction direction,
const Sensitivity sensitivity,
const til::point anchor) :
Expand Down Expand Up @@ -106,14 +106,13 @@ 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)
if (_coordSelEnd >= _coordSelStart)
{
_uiaData.ColorSelection(_coordSelStart, _coordSelEnd, attr);
}
Expand Down Expand Up @@ -330,7 +329,7 @@ void Search::_UpdateNextPosition()
// - wstr - String that will be our search term
// Return Value:
// - Structured text data for comparison to screen buffer text data.
std::vector<std::vector<wchar_t>> Search::s_CreateNeedleFromString(const std::wstring& wstr)
std::vector<std::vector<wchar_t>> Search::s_CreateNeedleFromString(const std::wstring_view wstr)
{
const auto charData = Utf16Parser::Parse(wstr);
std::vector<std::vector<wchar_t>> cells;
Expand Down
6 changes: 3 additions & 3 deletions src/buffer/out/search.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ class Search final
};

Search(Microsoft::Console::Types::IUiaData& uiaData,
const std::wstring& str,
const std::wstring_view str,
const Direction dir,
const Sensitivity sensitivity);

Search(Microsoft::Console::Types::IUiaData& uiaData,
const std::wstring& str,
const std::wstring_view str,
const Direction dir,
const Sensitivity sensitivity,
const til::point anchor);
Expand All @@ -68,7 +68,7 @@ class Search final

static til::point s_GetInitialAnchor(const Microsoft::Console::Types::IUiaData& uiaData, const Direction dir);

static std::vector<std::vector<wchar_t>> s_CreateNeedleFromString(const std::wstring& wstr);
static std::vector<std::vector<wchar_t>> s_CreateNeedleFromString(const std::wstring_view wstr);

bool _reachedEnd = false;
til::point _coordNext;
Expand Down
107 changes: 103 additions & 4 deletions 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,69 @@ 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
std::vector<til::point_span> TextBuffer::GetTextSpans(til::point start, til::point end, bool blockSelection, bool bufferCoordinates) const
{
std::vector<til::point_span> textSpans;
if (blockSelection)
{
// If blockSelection, this is effectively the same operation as GetTextRects, but
// expressed in til::point coordinates.
const auto rects = GetTextRects(start, end, /*blockSelection*/ true, bufferCoordinates);
textSpans.reserve(rects.size());

for (auto rect : rects)
{
const til::point first = { rect.Left, rect.Top };
const til::point second = { rect.Right, rect.Bottom };
textSpans.emplace_back(first, second);
}
}
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;

textSpans.emplace_back(higherCoord, lowerCoord);
}

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 @@ -1774,9 +1837,8 @@ const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF,
}
}
}
#pragma warning(suppress : 26444)
// TODO GH 2675: figure out why there's custom construction/destruction happening here
it++;

++it;
}

// We apply formatting to rows if the row was NOT wrapped or formatting of wrapped rows is allowed
Expand Down Expand Up @@ -1832,6 +1894,43 @@ const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF,
return data;
}

size_t TextBuffer::SpanLength(const til::point coordStart, const til::point coordEnd) const
{
const auto bufferSize = GetSize();
// The coords are inclusive, so to get the (inclusive) length we add 1.
const auto length = bufferSize.CompareInBounds(coordEnd, coordStart) + 1;
return gsl::narrow<size_t>(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.
std::wstring TextBuffer::GetPlainText(const til::point& start, const til::point& end) const
{
std::wstring text;
auto spanLength = SpanLength(start, end);
text.reserve(spanLength);

auto it = GetCellDataAt(start);

for (; it && spanLength > 0; ++it, --spanLength)
{
const auto& cell = *it;
if (!cell.DbcsAttr().IsTrailing())
{
const auto chars = cell.Chars();
text.append(chars);
}
}

return text;
}

// Routine Description:
// - Generates a CF_HTML compliant structure based on the passed in text and color data
// Arguments:
Expand Down
5 changes: 5 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;
std::vector<til::point_span> 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,16 @@ 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;

std::wstring GetPlainText(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
Loading

0 comments on commit d11ea72

Please sign in to comment.