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

Add termux support for fish_config. #6248

Merged
merged 1 commit into from Oct 26, 2019

Conversation

Liorst4
Copy link
Contributor

@Liorst4 Liorst4 commented Oct 25, 2019

No description provided.

@@ -49,6 +49,10 @@ def is_wsl():
return True
return False

def is_termux():
""" Return whether we are running under the Terumx application for Android"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terumx -> Termux

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -49,6 +49,10 @@ def is_wsl():
return True
return False

def is_termux():
""" Return whether we are running under the Terumx application for Android"""
return 'com.termux' in os.environ['PATH'] and 0 == os.system('which termux-open-url 1>&2 2>/dev/null')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which termux-open-url 1>&2 2>/dev/null will send stdout to stderr and stderr to /dev/null.
If you want to discard both, swap the order of the redirection operators.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this check itself is really quite.... unclean, and invokes another shell and the which command, which doesn't have to be installed and should be discouraged (in shell scripts, use command -v).

I'd suggest distutils.spawn.find_executable('termux-open-url'), which returns a path if it finds it and None if it doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using distutils instead of which

@Liorst4 Liorst4 requested review from krobelus and faho October 25, 2019 17:28
@faho faho added this to the fish 3.1.0 milestone Oct 26, 2019
@faho faho merged commit 5b22508 into fish-shell:master Oct 26, 2019
@faho
Copy link
Member

faho commented Oct 26, 2019

This looks much nicer now.

Merged, thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants