Trust but verify 'scutil' to return ComputerName #2743
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
ping? |
|
👍 |
| SHORT_HOST=$(scutil --get ComputerName) | ||
| if [[ "$OSTYPE" = darwin* ]]; then | ||
| # OS X's $HOST changes with dhcp, etc. Use ComputerName if possible. | ||
| SHORT_HOST=$(/usr/sbin/scutil --get ComputerName 2>/dev/null) || SHORT_HOST=${HOST/.*/} |
There was a problem hiding this comment.
scutil should continue being used without absolute path. Also, on #2183 they use `$(hostname -s)', is that better than using $HOST?
There was a problem hiding this comment.
There are three reasons to continue to use $HOST:
- Even if
$OSTYPEequalsdarwin*that doesn't mean they are running a full OS X environment (e.g. just running the darwin kernel on a bsd or gnu system).hostname -sdoesn't work on all unixes. $HOSTis faster since it doesn't have to fork/exec.- It matches what happens on non-OSX systems.
Apparently, it is possible to set up a Mac such that `scutil --get ComputerName` hasn't been set. This change checks if that fails and falls back to the original mechanism. Closes ohmyzsh#2155 Closes ohmyzsh#2183
|
I removed the |
|
Perfect, I support this change (bonus points for using |
|
If you want more @mcornella and @ncanceill -- Should I resubmit this and #2744 to https://github.com/ncanceill/oh-my-zsh/tree/easymerge ? |
|
/cc @robbyrussell |
robbyrussell
added a commit
that referenced
this pull request
Sep 3, 2014
Trust but verify 'scutil' to return ComputerName
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Apparently, it is possible to set up a Mac such that
scutil --get ComputerNamehasn't been set.This change checks if that fails and falls back to the original mechanism.
Closes #2155
Closes #2183