-
Notifications
You must be signed in to change notification settings - Fork 26
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
simple os-detection with shell script #7
base: master
Are you sure you want to change the base?
Conversation
. os-detect | ||
|
||
case "${grep_command}" in | ||
UNKNOWN*) echo "support only unix system";; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not put this part in the os-detect file with an exit?
It will not run anyway, and I think it would simplify the zkt{,-raw} scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just got the idea from you, just adding exit $? in the end of os-detect right?to be honest, my knowledge of shell scripts is lacking
OS_TYPE="$(uname -s)" | ||
case "${OS_TYPE}" in | ||
Linux*) sort_command=sort grep_command=grep;; | ||
Darwin*) sort_command=gsort grep_command=ggrep;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these don't exist, we'll need to print a warning to install them!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I was too late to respond to this. based on 000bbf7#r470224199 i think we could print a warning. and then do exit from os-detect file. is it good?
in response to issue #6, i wrote a simple and minimal way for this.