-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Fix console aliases not working #14991
Fix console aliases not working #14991
Conversation
@@ -20,7 +20,7 @@ bool INPUT_READ_HANDLE_DATA::IsInputPending() const | |||
|
|||
bool INPUT_READ_HANDLE_DATA::IsMultilineInput() const | |||
{ | |||
FAIL_FAST_IF(!_isInputPending); // we shouldn't have multiline input without a pending input. | |||
assert(_isInputPending); // we shouldn't have multiline input without a pending input. |
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.
Another one down the list #14298.
src/host/readDataCooked.cpp
Outdated
// This is a continuation of the above identical if condition. | ||
// We've truncated the `input` slice and now we need to restore it. | ||
const auto inputSizeAfter = input.size(); | ||
const auto amountConsumed = inputSizeBefore - inputSizeAfter; | ||
input = { _backupLimit, _bytesRead / sizeof(wchar_t) }; | ||
input = input.substr(amountConsumed); | ||
GetInputReadHandleData()->SaveMultilinePendingInput(input); |
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 isn't necessarily the most readable code I've written so far... But it does do the correct thing: After Consume()
returns, input
might not be empty! So if we restore the original, full input
slice we need to manually calculate the amount of data that was read (amountConsumed
).
An alternative I've considered is to change Consume()
to return the amount of characters that were read, but given how Consume()
works internally, this would be identical to this inputSizeBefore
/ inputSizeAfter
code. Alternatively we could pass a "character limit" argument, but that would be, once again, similar to the existing code, since string-views are already strings with a limit, so I felt like passing 2 limits wouldn't be any less confusing.
src/host/stream.cpp
Outdated
const auto inputSizeAfter = input.size(); | ||
const auto amountConsumed = inputSizeBefore - inputSizeAfter; | ||
input = pending.substr(amountConsumed); |
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 code block is identical to the other one, but it's shorter because we can easily access the previously saved pending
slice.
@miniksa thanks for selfhosting 😉 |
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.
Okay, this makes some sense to me.
I have to trust, to some extent, the code here. I understand it but only to a point. :D |
bfb11cd
to
1924a7e
Compare
// Don't be fooled by ProcessAliases only taking one argument. It rewrites multiple | ||
// class members on return, including `_bytesRead`, requiring us to reconstruct `input`. |
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.
Good callout!
#14745 contains two regressions related to console alias handling:
ProcessAliases
expands the backup buffer into (an) aliasedcommand(s) it changes the
_bytesRead
field ofCOOKED_READ_DATA
,requiring us to re-read it and reconstruct the
input
string-view.them any different from regular single-line inputs.
Validation Steps Performed
In
cmd.exe
runThe output should look exactly like this: