Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added new command for sending custom sequence to terminal #56962
Added new command for sending custom sequence to terminal #56962
Changes from 2 commits
9a384a4
6a6c809
a10e863
abde156
ea66a9f
6c20496
b642b3f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can we type
args
?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.
string
?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.
Prefer
let
tovar
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.
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.
Why the extra space?
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.
We need to add a space at the end or at the beginning (to separate text or any command).
If we keep a space at the end, it will have an extra space(near terminal prompt) in the new line after execution is over.
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.
Termina typo, also let's make this a private member of SendSequenceTerminalCommand
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.
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 needs to happen for every character, I suggest an algorithm like this:
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.
By
each character
, do you meaneach space separated word
? If so, this function is being called foreach space separated word
.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.
I think we have different ideas for the end solution, you want to pass in a space separated list of characters or words that translate to characters, I think the most flexible way would be to pass in a list of characters. So instead of this:
It would be:
I think we should start with this as it supports everything and then see if we need something better. In fact if we support my proposal an extension could provide the nicer interface if needed.
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.
I could see something like this being nice:
But I think it should live in an extension first to see if people find it useful.
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.
On implementation side, implementing this is easy:
"args": {"text": "\x03\x1b[A\x0d"}
For each char, termnalInstance.sendText(String.fromChar(char))
But will it be easy enough for end user to use?
Final decision is yours. Let me know what way should we go :)
BTW, How should we handle
[A
if we go that way?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.
Sorry about the delay, had too many notifications 😱
\a
,\b
, etc. to work as specified in this file: https://github.com/xtermjs/xterm.js/blob/f47f095b8928a6e50ce80f87d7fd11e50cf9a190/src/common/data/EscapeSequences.ts#L25, not sure if they will without special casing{ "text": "hello world" }
should send that, including the space\\x03
in the keybindings.jsonThere 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.
@Tyriar
I'm stuck on one last thing, that I need some help with,
If I use
\\x03
forctrl+c
, it works but thenterminalInstance.sendText
ignores next single char.That is, for
args: {'text': '\\x03ls\n'}
, the output on terminal isbash: s: command not found
It works as expected with
args: {'text': '\\x03 ls\n'}
For
upArrow
key, if I use"args": {"text": "\\x03\\x1b[A\n"}
or"args": {"text": "\\x03 \\x1b[A\n"}
I'm gettingbash: s: command not found
on terminal. (last executed command is ls.)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.
I just tested sending directly to the process in the xterm.js demo (what VS Code's terminal is based on) and sending
\x03
definitely doesn't eat the next character for me: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.
Can you please try
term._core.handler("\x03ls\n")
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.
I can repro this in Linux, I'm guessing this is expected behavior of bash. For this I would try workaround it by using something like
\r
to make sure the ^C goes though which apppears to work\x03\rls\n
, it adds another line but that's not a big deal.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.
I'm not sure these key strings should be handled specially, what if the user wants to send the text "down"?
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.
Should I remove all except
up
?