Skip to content

ssh/terminal: Use move-N sequences for >1 cursor moves #82

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

ssh/terminal: Use move-N sequences for >1 cursor moves #82

wants to merge 1 commit into from

Conversation

shazow
Copy link
Contributor

@shazow shazow commented Mar 24, 2019

Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

@gopherbot
Copy link
Contributor

This PR (HEAD: 02cc180) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/169077 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Andrey Petrov:

Patch Set 1:

A few notes:

  • I've been using this change for a while in one of my projects (ssh-chat), so I already had the code ready to go.
  • There is no related issue about this, but the work has already been done so I hope we can discuss over the PR.
  • If there are consumers who rely on the old output format for some reason, this could break them. Though it's hard to imagine a legitimate use case within the ecosystem. For ssh-chat, we had some third-party bots that read the produced output and responded based on that, so they had to adjust their parsing. Overall I feel this change is worth it.

Please don’t reply on this GitHub thread. Visit golang.org/cl/169077.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 1: Run-TryBot+1 Code-Review+2

R=agl

Adam, any objections?

Please don’t reply on this GitHub thread. Visit golang.org/cl/169077.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=8a1dc27f

Please don’t reply on this GitHub thread. Visit golang.org/cl/169077.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Andrey Petrov:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/169077.
After addressing review feedback, remember to publish your drafts!

@shazow
ssh/terminal: Use move-N sequences for >1 cursor moves
92ef253 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.
@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/169077.
After addressing review feedback, remember to publish your drafts!

@shazow shazow force-pushed the terminal-multimove branch from 02cc180 to 92ef253 Compare March 25, 2019 14:55
@gopherbot
Copy link
Contributor

This PR (HEAD: 92ef253) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/169077 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Andrey Petrov:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/169077.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Adam Langley:

Patch Set 2: Code-Review+2

If this is widely supported by terminals then LGTM.

Please don’t reply on this GitHub thread. Visit golang.org/cl/169077.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Mar 25, 2019
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
a5d413f 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef253
GitHub-Pull-Request: #82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/169077 has been merged.

@gopherbot gopherbot closed this Mar 25, 2019
@thaJeztah thaJeztah mentioned this pull request Apr 6, 2019
Merged
bored-engineer pushed a commit to bored-engineer/ssh that referenced this pull request Oct 13, 2019
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
e63ce44 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this pull request Oct 13, 2019
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
6129144 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this pull request Oct 13, 2019
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
609d25d 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
gopherbot pushed a commit to golang/term that referenced this pull request Oct 16, 2020
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
c5b4c79 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
jandd pushed a commit to jandd/crypto that referenced this pull request Jun 26, 2021
@marinthiercelin
Allow encrypting text messages to split writers (golang#82)
c05353c
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this pull request Mar 28, 2022
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
ca06166 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this pull request Mar 29, 2022
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
d00e346 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this pull request Mar 29, 2022
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
6aa095f 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this pull request Feb 16, 2023
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
c903b80 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
BiiChris pushed a commit to BiiChris/crypto that referenced this pull request Sep 15, 2023
@shazow @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
caf1037 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef253
GitHub-Pull-Request: golang#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
desdeel2d0m added a commit to desdeel2d0m/crypto that referenced this pull request Jul 1, 2024
@desdeel2d0m @bradfitz
ssh/terminal: Use move-N sequences for >1 cursor moves
5112494 
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shazow @gopherbot @googlebot