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

Fix console disconnection #6517

Merged
merged 4 commits into from Nov 27, 2019
Merged

Fix console disconnection #6517

merged 4 commits into from Nov 27, 2019

Conversation

@stgraber
Copy link
Member

stgraber commented Nov 27, 2019

No description provided.

stgraber added 4 commits Nov 27, 2019
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
Closes #6512

Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
@brauner brauner merged commit f98cdb1 into lxc:master Nov 27, 2019
4 of 5 checks passed
4 of 5 checks passed
Testsuite Build finished.
Details
Branch target Branch target is correct
Details
DCO All commits signed-off
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tomponline

This comment has been minimized.

Copy link
Member

tomponline commented on shared/network.go in 880fca0 Nov 27, 2019

@stgraber why is this needed out of interest?

This comment has been minimized.

Copy link
Member Author

stgraber replied Nov 27, 2019

That's so that when we show an error (which is usually what that code path is used for), we don't leave the console in a messed up state.

So we effectively had two cases to fix in this branch:

  • Clean disconnects (ctrl a+q) => insert new line and move cursor to beginning
  • Disconnect due to error => error inserts a line break but the cursor may be mis-placed so reset it
s.connsLock.Unlock()
consolConn.Close()
consoleConn.WriteMessage(websocket.BinaryMessage, []byte("\n\r"))

This comment has been minimized.

Copy link
@tomponline

tomponline Nov 27, 2019

Member

@stgraber the \n\r sequence is quite unusual, is that on purpose? And why is that different to the \r sent further down? Thanks

This comment has been minimized.

Copy link
@stgraber

stgraber Nov 27, 2019

Author Member

Yeah, it feels a bit odd but just \n would insert a new line but still cause the cursor to be way mis-aligned.
\n\r causes a line break and resets the cursor to the beginning cleaning things up nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.