Skip to content
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 an experimental setting for moving the cursor with the mouse #15758

Merged
merged 10 commits into from Aug 14, 2023
21 changes: 21 additions & 0 deletions src/buffer/out/textBuffer.cpp
Expand Up @@ -246,6 +246,27 @@ til::CoordType TextBuffer::TotalRowCount() const noexcept
return _height;
}

// Method Description:
// - Gets the number of glyphs in the buffer between two points.
// - IMPORTANT: Make sure that start is before end, or this will never return!
// Arguments:
// - start - The starting point of the range to get the glyph count for.
// - end - The ending point of the range to get the glyph count for.
// Return Value:
// - The number of glyphs in the buffer between the two points.
size_t TextBuffer::GetCellDistance(const til::point from, const til::point to) const
{
auto startCell = GetCellDataAt(from);
const auto endCell = GetCellDataAt(to);
auto delta = 0;
while (startCell != endCell)
{
++startCell;
++delta;
}
return delta;
lhecker marked this conversation as resolved.
Show resolved Hide resolved
}

// Routine Description:
// - Retrieves read-only text iterator at the given buffer location
// Arguments:
Expand Down
2 changes: 2 additions & 0 deletions src/buffer/out/textBuffer.hpp
Expand Up @@ -131,6 +131,8 @@ class TextBuffer final
TextBufferTextIterator GetTextLineDataAt(const til::point at) const;
TextBufferTextIterator GetTextDataAt(const til::point at, const Microsoft::Console::Types::Viewport limit) const;

size_t GetCellDistance(const til::point from, const til::point to) const;

static size_t GraphemeNext(const std::wstring_view& chars, size_t position) noexcept;
static size_t GraphemePrev(const std::wstring_view& chars, size_t position) noexcept;

Expand Down
53 changes: 51 additions & 2 deletions src/cascadia/TerminalControl/ControlCore.cpp
Expand Up @@ -1447,7 +1447,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
viewHeight,
bufferSize) };

if (_inUnitTests) [[unlikely]]
if (_inUnitTests)
[[unlikely]]
{
_ScrollPositionChangedHandlers(*this, update);
}
Expand Down Expand Up @@ -1751,7 +1752,55 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_terminal->MultiClickSelection(terminalPosition, mode);
selectionNeedsToBeCopied = true;
}
_updateSelectionUI();
else if (_settings->MoveCursorWithMouse()) // This is also mode==Char && !shiftEnabled
{
// If we're handling a single left click, without shift pressed, and
// outside mouse mode, AND the user has MoveCursorWithMouse turned
// on, let's try to move the cursor.
//
// We'll only move the cursor if the user has clicked after the last
// mark, if there is one. That means the user also needs to set up
// shell integration to enable this feature.
//
// As noted in GH #8573, there's plenty of edge cases with this
// approach, but it's good enough to bring value to 90% of use cases.
const auto cursorPos{ _terminal->GetCursorPosition() };

// Does the current buffer line have a mark on it?
const auto& marks{ _terminal->GetScrollMarks() };
if (!marks.empty())
{
const auto& last{ marks.back() };
const auto [start, end] = last.GetExtent();
if (terminalPosition >= end)
{
// Get the distance between the cursor and the click, in cells.
const auto bufferSize = _terminal->GetTextBuffer().GetSize();

// First, make sure to iterate from the first point to the
// second. The user may have clicked _earlier_ in the
// buffer!
auto goRight = terminalPosition > cursorPos;
const auto startPoint = goRight ? cursorPos : terminalPosition;
const auto endPoint = goRight ? terminalPosition : cursorPos;

const auto delta = _terminal->GetTextBuffer().GetCellDistance(startPoint, endPoint);

const WORD key = goRight ? VK_RIGHT : VK_LEFT;
// Send an up and a down once per cell. This won't
// accurately handle wide characters, or continuation
// prompts, or cases where a single escape character in the
// command (e.g. ^[) takes up two cells.
for (size_t i = 0u; i < delta; i++)
{
_terminal->SendKeyEvent(key, 0, {}, true);
_terminal->SendKeyEvent(key, 0, {}, false);
Comment on lines +1815 to +1816
Copy link
Member

@lhecker lhecker Jul 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For purposes like this it would be nice if we could stop flushing our input pipe so that the keystrokes are sent as a unison. For both pwsh and cmd this would avoid re-printing the entire commandline on every cursor navigation. One option would be to do something like _terminal->CorkInputPipe() and UncorkInputPipe() (borrowing TCP_CORK terminology), but I also somewhat feel like it would be better if we instead made it so that SendKeyEvent will never flush the input pipe and instead we add a _terminal->FlushInput() function which you need to call when you're done generating input. Alternatively, we could also extract TerminalInput out of Terminals control and accumulate a std::string here before sending it off. Or we just generate cursor sequences ourselves. I don't think there's any fantastic solution, but there's a lot of alright ones. 🙂

(xterm.js does this as well btw.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we create a follow-up issue for this? I think it would be worthwhile in general to keep track of features that would benefit of corking/uncorking the input pipe.

}
}
}

_updateSelectionUI();
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Method Description:
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/IControlSettings.idl
Expand Up @@ -61,5 +61,6 @@ namespace Microsoft.Terminal.Control
Boolean ShowMarks { get; };
Boolean UseBackgroundImageForWindow { get; };
Boolean RightClickContextMenu { get; };
Boolean MoveCursorWithMouse { get; };
};
}
9 changes: 8 additions & 1 deletion src/cascadia/TerminalSettingsModel/MTSMSettings.h
Expand Up @@ -68,6 +68,12 @@ Author(s):
X(bool, IsolatedMode, "compatibility.isolatedMode", false) \
X(hstring, SearchWebDefaultQueryUrl, "searchWebDefaultQueryUrl", L"https://www.bing.com/search?q=%22%s%22")

// Also add these settings to:
// * Profile.idl
// * TerminalSettings.h
// * TerminalSettings.cpp: TerminalSettings::_ApplyProfileSettings
// * IControlSettings.idl or ICoreSettings.idl
// * ControlProperties.h
#define MTSM_PROFILE_SETTINGS(X) \
X(int32_t, HistorySize, "historySize", DEFAULT_HISTORY_SIZE) \
X(bool, SnapOnInput, "snapOnInput", true) \
Expand All @@ -90,7 +96,8 @@ Author(s):
X(bool, Elevate, "elevate", false) \
X(bool, VtPassthrough, "experimental.connection.passthroughMode", false) \
X(bool, AutoMarkPrompts, "experimental.autoMarkPrompts", false) \
X(bool, ShowMarks, "experimental.showMarksOnScrollbar", false)
X(bool, ShowMarks, "experimental.showMarksOnScrollbar", false) \
X(bool, MoveCursorWithMouse, "experimental.moveCursorWithMouse", false)

// Intentionally omitted Profile settings:
// * Name
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/Profile.idl
Expand Up @@ -94,5 +94,6 @@ namespace Microsoft.Terminal.Settings.Model
INHERITABLE_PROFILE_SETTING(Boolean, ShowMarks);

INHERITABLE_PROFILE_SETTING(Boolean, RightClickContextMenu);
INHERITABLE_PROFILE_SETTING(Boolean, MoveCursorWithMouse);
}
}
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.cpp
Expand Up @@ -325,6 +325,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
_ShowMarks = Feature_ScrollbarMarks::IsEnabled() && profile.ShowMarks();

_RightClickContextMenu = profile.RightClickContextMenu();

_MoveCursorWithMouse = profile.MoveCursorWithMouse();
}

// Method Description:
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.h
Expand Up @@ -164,6 +164,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
INHERITABLE_SETTING(Model::TerminalSettings, bool, AutoMarkPrompts, false);
INHERITABLE_SETTING(Model::TerminalSettings, bool, ShowMarks, false);
INHERITABLE_SETTING(Model::TerminalSettings, bool, RightClickContextMenu, false);
INHERITABLE_SETTING(Model::TerminalSettings, bool, MoveCursorWithMouse, false);

private:
std::optional<std::array<Microsoft::Terminal::Core::Color, COLOR_TABLE_SIZE>> _ColorTable;
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/inc/ControlProperties.h
Expand Up @@ -47,7 +47,8 @@
X(winrt::hstring, StartingTitle) \
X(bool, DetectURLs, true) \
X(bool, VtPassthrough, false) \
X(bool, AutoMarkPrompts)
X(bool, AutoMarkPrompts) \
X(bool, MoveCursorWithMouse, false)

// --------------------------- Control Settings ---------------------------
// All of these settings are defined in IControlSettings.
Expand Down