Skip to content

Add support for bracketed paste mode in ConHost #15155

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Add support for bracketed paste mode in ConHost #15155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/Terminal.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class Microsoft::Terminal::Core::Terminal final :
void SetConsoleOutputCP(const unsigned int codepage) noexcept override;
unsigned int GetConsoleOutputCP() const noexcept override;
void SetBracketedPasteMode(const bool enabled) noexcept override;
std::optional<bool> GetBracketedPasteMode() const noexcept override;
bool GetBracketedPasteMode() const noexcept override;
void CopyToClipboard(std::wstring_view content) override;
void SetTaskbarProgress(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::TaskbarState state, const size_t progress) override;
void SetWorkingDirectory(std::wstring_view uri) override;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/TerminalApi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void Terminal::SetBracketedPasteMode(const bool enabled) noexcept
_bracketedPasteMode = enabled;
}

std::optional<bool> Terminal::GetBracketedPasteMode() const noexcept
bool Terminal::GetBracketedPasteMode() const noexcept
{
return _bracketedPasteMode;
}
Expand Down
41 changes: 10 additions & 31 deletions src/host/consoleInformation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,37 +18,6 @@
using Microsoft::Console::Interactivity::ServiceLocator;
using Microsoft::Console::VirtualTerminal::VtIo;

CONSOLE_INFORMATION::CONSOLE_INFORMATION() :
// ProcessHandleList initializes itself
pInputBuffer(nullptr),
pCurrentScreenBuffer(nullptr),
ScreenBuffers(nullptr),
OutputQueue(),
// ExeAliasList initialized below
_OriginalTitle(),
_Title(),
_Prefix(),
_TitleAndPrefix(),
_LinkTitle(),
Flags(0),
PopupCount(0),
CP(0),
OutputCP(0),
CtrlFlags(0),
LimitingProcessId(0),
// ColorTable initialized below
// CPInfo initialized below
// OutputCPInfo initialized below
_cookedReadData(nullptr),
ConsoleIme{},
_vtIo(),
_blinker{},
renderData{}
{
ZeroMemory((void*)&CPInfo, sizeof(CPInfo));
ZeroMemory((void*)&OutputCPInfo, sizeof(OutputCPInfo));
}

bool CONSOLE_INFORMATION::IsConsoleLocked() const noexcept
{
return _lock.is_locked();
Expand Down Expand Up @@ -176,6 +145,16 @@ void CONSOLE_INFORMATION::SetCookedReadData(COOKED_READ_DATA* readData) noexcept
_cookedReadData = readData;
}

bool CONSOLE_INFORMATION::GetBracketedPasteMode() const noexcept
{
return _bracketedPasteMode;
}

void CONSOLE_INFORMATION::SetBracketedPasteMode(const bool enabled) noexcept
{
_bracketedPasteMode = enabled;
}

// Method Description:
// - Return the active screen buffer of the console.
// Arguments:
Expand Down
12 changes: 4 additions & 8 deletions src/host/outputStream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,22 +254,18 @@ unsigned int ConhostInternalGetSet::GetConsoleOutputCP() const
// - <none>
void ConhostInternalGetSet::SetBracketedPasteMode(const bool enabled)
{
// TODO GH#395: Bracketed Paste Mode is not yet supported in conhost, but we
// still keep track of the state so it can be reported by DECRQM.
_bracketedPasteMode = enabled;
ServiceLocator::LocateGlobals().getConsoleInformation().SetBracketedPasteMode(enabled);
}

// Routine Description:
// - Gets the current state of XTerm bracketed paste mode.
// Arguments:
// - <none>
// Return Value:
// - true if the mode is enabled, false if not, nullopt if unsupported.
std::optional<bool> ConhostInternalGetSet::GetBracketedPasteMode() const
// - true if the mode is enabled, false if not.
bool ConhostInternalGetSet::GetBracketedPasteMode() const
{
// TODO GH#395: Bracketed Paste Mode is not yet supported in conhost, so we
// only report the state if we're tracking it for conpty.
return IsConsolePty() ? std::optional{ _bracketedPasteMode } : std::nullopt;
return ServiceLocator::LocateGlobals().getConsoleInformation().GetBracketedPasteMode();
}

// Routine Description:
Expand Down
3 changes: 1 addition & 2 deletions src/host/outputStream.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
unsigned int GetConsoleOutputCP() const override;

void SetBracketedPasteMode(const bool enabled) override;
std::optional<bool> GetBracketedPasteMode() const override;
bool GetBracketedPasteMode() const override;
void CopyToClipboard(const std::wstring_view content) override;
void SetTaskbarProgress(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::TaskbarState state, const size_t progress) override;
void SetWorkingDirectory(const std::wstring_view uri) override;
Expand All @@ -80,5 +80,4 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::

private:
Microsoft::Console::IIoProvider& _io;
bool _bracketedPasteMode{ false };
};
30 changes: 17 additions & 13 deletions src/host/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,29 +79,29 @@ class CONSOLE_INFORMATION :
public Microsoft::Console::IIoProvider
{
public:
CONSOLE_INFORMATION();
CONSOLE_INFORMATION() = default;
CONSOLE_INFORMATION(const CONSOLE_INFORMATION& c) = delete;
CONSOLE_INFORMATION& operator=(const CONSOLE_INFORMATION& c) = delete;

ConsoleProcessList ProcessHandleList;
InputBuffer* pInputBuffer;
InputBuffer* pInputBuffer = nullptr;

SCREEN_INFORMATION* ScreenBuffers; // singly linked list
SCREEN_INFORMATION* ScreenBuffers = nullptr; // singly linked list
ConsoleWaitQueue OutputQueue;

DWORD Flags;
DWORD Flags = 0;

std::atomic<WORD> PopupCount;
std::atomic<WORD> PopupCount = 0;

// the following fields are used for ansi-unicode translation
UINT CP;
UINT OutputCP;
UINT CP = 0;
UINT OutputCP = 0;

ULONG CtrlFlags; // indicates outstanding ctrl requests
ULONG LimitingProcessId;
ULONG CtrlFlags = 0; // indicates outstanding ctrl requests
ULONG LimitingProcessId = 0;

CPINFO CPInfo;
CPINFO OutputCPInfo;
CPINFO CPInfo = {};
CPINFO OutputCPInfo = {};
Copy link
Member

Choose a reason for hiding this comment

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

chef's kiss


ConsoleImeInfo ConsoleIme;

Expand All @@ -125,6 +125,9 @@ class CONSOLE_INFORMATION :
COOKED_READ_DATA& CookedReadData() noexcept;
void SetCookedReadData(COOKED_READ_DATA* readData) noexcept;

bool GetBracketedPasteMode() const noexcept;
void SetBracketedPasteMode(const bool enabled) noexcept;

void SetTitle(const std::wstring_view newTitle);
void SetTitlePrefix(const std::wstring_view newTitlePrefix);
void SetOriginalTitle(const std::wstring_view originalTitle);
Expand Down Expand Up @@ -155,8 +158,9 @@ class CONSOLE_INFORMATION :
std::wstring _TitleAndPrefix;
std::wstring _OriginalTitle;
std::wstring _LinkTitle; // Path to .lnk file
SCREEN_INFORMATION* pCurrentScreenBuffer;
COOKED_READ_DATA* _cookedReadData; // non-ownership pointer
SCREEN_INFORMATION* pCurrentScreenBuffer = nullptr;
COOKED_READ_DATA* _cookedReadData = nullptr; // non-ownership pointer
bool _bracketedPasteMode = false;
Copy link
Member

Choose a reason for hiding this comment

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

I am so glad you made this change. Thanks. I mean, all of the initializers.


Microsoft::Console::VirtualTerminal::VtIo _vtIo;
Microsoft::Console::CursorBlinker _blinker;
Expand Down
30 changes: 27 additions & 3 deletions src/interactivity/win32/Clipboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ void Clipboard::StringPaste(_In_reads_(cchData) const wchar_t* const pData,

try
{
auto inEvents = TextToKeyEvents(pData, cchData);
const auto vtInputMode = gci.pInputBuffer->IsInVirtualTerminalInputMode();
const auto bracketedPasteMode = gci.GetBracketedPasteMode();
auto inEvents = TextToKeyEvents(pData, cchData, vtInputMode && bracketedPasteMode);
gci.pInputBuffer->Write(inEvents);
}
catch (...)
Expand All @@ -127,16 +129,31 @@ void Clipboard::StringPaste(_In_reads_(cchData) const wchar_t* const pData,
// Arguments:
// - pData - the text to convert
// - cchData - the size of pData, in wchars
// - bracketedPaste - should this be bracketed with paste control sequences
// Return Value:
// - deque of KeyEvents that represent the string passed in
// Note:
// - will throw exception on error
std::deque<std::unique_ptr<IInputEvent>> Clipboard::TextToKeyEvents(_In_reads_(cchData) const wchar_t* const pData,
const size_t cchData)
const size_t cchData,
const bool bracketedPaste)
{
THROW_HR_IF_NULL(E_INVALIDARG, pData);

std::deque<std::unique_ptr<IInputEvent>> keyEvents;
const auto pushControlSequence = [&](const std::wstring_view sequence) {
std::for_each(sequence.begin(), sequence.end(), [&](const auto wch) {
Copy link
Member

Choose a reason for hiding this comment

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

A range-for should be simpler I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The main reason I used std::for_each was so I could avoid having to put the { on a new line, which I really loathe. I know that's petty. But I did also check out the generated assembly, and std::for_each actually seemed to be a little more efficient - at least fewer instructions - so I figured that might count as a more rational justification.

That said, it's not a big deal to change if you really don't like it this way.

Copy link
Member

@lhecker lhecker Apr 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The main reason I used std::for_each was so I could avoid having to put the { on a new line, which I really loathe. I know that's petty.

If you knew. 🥲 K&R gang 4 life.

But I did also check out the generated assembly, and std::for_each actually seemed to be a little more efficient - at least fewer instructions - so I figured that might count as a more rational justification.

Oh interesting. I was always under the impression that compilers produce better assembly for range-for loops if they can infer the bounds easily (like for span/string_view). At least in my little test it produces the same assembly though: https://godbolt.org/z/crj8aM1qr (it's the function next to the pushControlSequence$ = 96). BTW you can also spot the 4 SSE2 movaps/movdqa there, which exist due to taking the string-view by-copy. It's quite sad to see that the latest MSVC actually still hasn't fixed this issue... The movaps/movdqa pairs are entirely redundant, because all it does is copy from one spot on the stack to another. Just read the damn original value! 😄 The movaps/movdqa sequence has a latency of like 10-15 cycles.

That said, it's not a big deal to change if you really don't like it this way.

No, please feel free to keep it (unless anyone else insists on it of course). Although you might be interested in using std::ranges::for_each(sequence, ...) (produces the same assembly, but it's more template instantiations unfortunately).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

K&R gang 4 life.

Absolutely!

At least in my little test it produces the same assembly though: https://godbolt.org/z/crj8aM1qr

That is interesting. That looks nothing like the assembly I'm seeing generated in Visual Studio. I don't think my version of VS is that out of date (the C/C++ Compiler is 19.34.31937), but maybe it's just the other code in the method that alters the way those lambdas are optimized.

Not that I'm seeing a huge difference between the two anyway. I just found it interesting that the std::for_each seemed a little better. I was concerned it would be worse.

Although you might be interested in using std::ranges::for_each(sequence, ...)

Yes, I was hoping there was something like that, so I'm glad you brought that up. Unfortunately it also introduces additional overhead in the generated assembly. For the most part it looks almost identical to the std::for_each version, but for some reason it adds a security_cookie, and an associated security_check_cookie call on exit. That's really annoying.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On looking at your godbolt link again, I realised now why it didn't look anything like what I was expecting. The function immediately below pushControlSequence$ = 96 (which is identical for both versions), is just the code that calls the pushControlSequence lambda. I think the lambda itself is further below that (following sequence$ = 88 in the one source, and sequence$ = 104 in the other).

In godbolt those too lambdas are massively different, but that's mostly because the one seems to have partially inlined the push_back calls. I don't get that inlining in VS for either of them, but the general structure is similar to what I'm seeing. The differences I see in VS are things like the one having more registers saved on the stack, and extra register loads in the body of the loop.

keyEvents.push_back(std::make_unique<KeyEvent>(true, 1ui16, 0ui16, 0ui16, wch, 0));
keyEvents.push_back(std::make_unique<KeyEvent>(false, 1ui16, 0ui16, 0ui16, wch, 0));
});
};

// When a bracketed paste is requested, we need to wrap the text with
// control sequences which indicate that the content has been pasted.
if (bracketedPaste)
{
pushControlSequence(L"\x1b[200~");
}

for (size_t i = 0; i < cchData; ++i)
{
Expand All @@ -148,8 +165,10 @@ std::deque<std::unique_ptr<IInputEvent>> Clipboard::TextToKeyEvents(_In_reads_(c
const auto skipLinefeed = (i != 0 &&
currentChar == UNICODE_LINEFEED &&
pData[i - 1] == UNICODE_CARRIAGERETURN);
// filter out escape if bracketed paste mode is enabled
const auto skipEscape = (bracketedPaste && currentChar == UNICODE_ESC);

if (!charAllowed || skipLinefeed)
if (!charAllowed || skipLinefeed || skipEscape)
{
continue;
}
Expand Down Expand Up @@ -182,6 +201,11 @@ std::deque<std::unique_ptr<IInputEvent>> Clipboard::TextToKeyEvents(_In_reads_(c
convertedEvents.pop_front();
}
}

if (bracketedPaste)
{
pushControlSequence(L"\x1b[201~");
}
return keyEvents;
}

Expand Down
3 changes: 2 additions & 1 deletion src/interactivity/win32/clipboard.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ namespace Microsoft::Console::Interactivity::Win32

private:
std::deque<std::unique_ptr<IInputEvent>> TextToKeyEvents(_In_reads_(cchData) const wchar_t* const pData,
const size_t cchData);
const size_t cchData,
const bool bracketedPaste = false);

void StoreSelectionToClipboard(_In_ const bool fAlsoCopyFormatting);

Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/ITerminalApi.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ namespace Microsoft::Console::VirtualTerminal
virtual unsigned int GetConsoleOutputCP() const = 0;

virtual void SetBracketedPasteMode(const bool enabled) = 0;
virtual std::optional<bool> GetBracketedPasteMode() const = 0;
virtual bool GetBracketedPasteMode() const = 0;
virtual void CopyToClipboard(const std::wstring_view content) = 0;
virtual void SetTaskbarProgress(const DispatchTypes::TaskbarState state, const size_t progress) = 0;
virtual void SetWorkingDirectory(const std::wstring_view uri) = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/terminal/adapter/ut_adapter/adapterTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,10 @@ class TestGetSet final : public ITerminalApi
Log::Comment(L"SetBracketedPasteMode MOCK called...");
}

std::optional<bool> GetBracketedPasteMode() const override
bool GetBracketedPasteMode() const override
{
Log::Comment(L"GetBracketedPasteMode MOCK called...");
return {};
return false;
}

void CopyToClipboard(const std::wstring_view /*content*/)
Expand Down