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

Support locales in ping output #57

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

Conversation

pixelbrackets
Copy link
Contributor

@pixelbrackets pixelbrackets commented Aug 6, 2020

Loosen the regex to search for a time value, to support non-english output of the ping command.

Since the output is generic in all languages we dont need to look for a specific english term.

This avoids introducing translations or keyword flags and so on.

Tested in German and English. I looked for other languages as well, almost all languages with latin characters use "ms" as abbreviation for milliseconds.

German
export LANG=de_DE.UTF-8
PING gitlab.com (0.0.0.0) 56(84) Bytes Daten.64 Bytes von 0.0.0.0: icmp_seq=1 ttl=58 Zeit=19.0 ms--- gitlab.com ping statistics ---1 Pakete übertragen, 1 empfangen, 0% Paketverlust, Zeit 0msrtt min/avg/max/mdev = 19.028/19.028/19.028/0.000 msHost could not be reached.

English
export LANG=en_US.UTF-8
PING gitlab.com (0.0.0.0) 56(84) bytes of data.64 bytes from 0.0.0.0: icmp_seq=1 ttl=58 time=19.6 ms--- gitlab.com ping statistics ---1 packets transmitted, 1 received, 0% packet loss, time 0msrtt min/avg/max/mdev = 19.605/19.605/19.605/0.000 msLatency is 19.6 ms         

Issue #49 shows that this works in Chinese as well.

Unit Test were red before on my machine, green now. 🎉

Refs #5 Refs #49

btw: Thanks for this package :)

Loosen the regex to search for a time value,
to support non-english output of the ping command.

Since the output is generic in all languages we dont
need to look for a specific english term.

Refs geerlingguy#5 Refs geerlingguy#49
@stale
Copy link

stale bot commented Nov 4, 2020

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

@stale stale bot added the stale label Nov 4, 2020
@pixelbrackets
Copy link
Contributor Author

Just a sidenote: this PR was marked as stale, the referring issue #5 was marked as planned however.

@stale
Copy link

stale bot commented Nov 4, 2020

This issue is no longer marked for closure.

@stale stale bot removed the stale label Nov 4, 2020
@geerlingguy
Copy link
Owner

Added the label. I will try to get this merged after I do a little extra repo maintenance. Thanks for your patience!

@jamieblomerus
Copy link

@geerlingguy
Have you found the time to review this PR yet? I just want to give a small reminder since the last message was more than 2 years ago.

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

Successfully merging this pull request may close these issues.

3 participants