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

Name the server on timeout banner #1055

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tansly
Copy link

@tansly tansly commented Jul 17, 2019

Fixes #1049.

Shows the server's IP address on the timeout banner.

I can improve this if necessary. A thing that crosses my mind:

  • This still can be confusing for the user because the user may have started mosh using a domain name thus does not necessarily know the IP address of the server. For better UX, I may try to display what the user gave as an argument when starting mosh. But this may be a problem if the hostname is overly long and does not fit in the banner.

@tansly tansly force-pushed the timeout-show-host branch 4 times, most recently from 616f6f1 to e70a888 Compare July 17, 2019 10:07
@tansly
Copy link
Author

tansly commented Jul 17, 2019

Fixed errors in CI.

Edit: Apparently not. Something strange is going on: OSX tests fail but it seems totally unrelated; because on all of my other pushes the Linux build failed but the OSX build passed.

Here, Linux build fails but OSX passes: https://travis-ci.org/mobile-shell/mosh/builds/559884676. The commit is 39aa6be.
But here, Linux build passes but OSX build fails: https://travis-ci.org/mobile-shell/mosh/builds/559885685. The commit is e2f9d06.
The difference between these commits is here: https://github.com/mobile-shell/mosh/compare/39aa6be8cd0d309ee435dc003aa4fe61edb53b00..e2f9d06ec976f6269d96936b0992e6b2b0c03db4

Edit2: Build passes now, without changing anything.

@tansly tansly force-pushed the timeout-show-host branch from 47c43a6 to e70a888 Compare July 17, 2019 14:20
@tansly tansly force-pushed the timeout-show-host branch from e70a888 to d2f1266 Compare July 17, 2019 14:26
@Kriechi
Copy link

Kriechi commented Nov 16, 2019

@andersk @cgull anything holding this PR back?

@daonb
Copy link

daonb commented Apr 3, 2020

Hi all, I'm new here. Started using blink on an iPad and fell in love with mosh. I've been using two server: a local one in TLV and one from my client in us-east-1 and mosh just works.

My two cents is that the code looks good, but it has no tests. Probably because this message is not covered in the current e2e tests. Could be a good time to add this test case.

@tansly
Copy link
Author

tansly commented Apr 9, 2020

Hi all, I'm new here. Started using blink on an iPad and fell in love with mosh. I've been using two server: a local one in TLV and one from my client in us-east-1 and mosh just works.

My two cents is that the code looks good, but it has no tests. Probably because this message is not covered in the current e2e tests. Could be a good time to add this test case.

Hi @daonb, I would work on it but the latest commit to this repo is about 10 months ago. I’m not sure that it is active right now. I’m holding this until there is some activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mosh timeout banner does not name the host(s) on the connection
3 participants