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

Make help and fish_config work on Chrome OS #7789

Merged
merged 2 commits into from Mar 7, 2021
Merged

Make help and fish_config work on Chrome OS #7789

merged 2 commits into from Mar 7, 2021

Conversation

ilyagr
Copy link
Contributor

@ilyagr ilyagr commented Mar 7, 2021

When fish is running in the Chrome OS Linux VM (Crostini), both help and fish_config currently open a "file not found"
page. That is because on Crostini, BROWSER is usually set to garcon-url-handler, which opens URLs in the host OS Chrome
browser. That browser lacks access to the Linux file system.

This PR fixes these commands. help now opens the URL on www.fishshell.com. fish_config now opens the URL for the
server it starts. Previously, it opened a local file that redirects to the same URL.

In the case of help, the situation could be improved further by starting a web server to serve help. I don't know of another way to access /share/fish from outside the VM without user intervention, and I think that might be a part of the security
model for the Crostini VM.

It's hard to write a test for this. I checked that help math, python2 webconfig.py, and python3 webconfig.py work on my
machine running in Crostini.


I also rearranged the imports a little to group them together. It's in a separate commit, let me know if you prefer that I not do that.

Another alternative is to look much harder for a browser installed in the Linux VM. By default, none are installed, I think. I do think a Linux browser should be used if the user has installed it and put it into BROWSER, though.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@ilyagr ilyagr changed the title Chromeos browser Make help and fish_config work on Chrome OS Mar 7, 2021
When `fish` is running in the Chrome OS Linux VM (Crostini),
both `help` and `fish_config` opened a "file not found"
page. That is because on Crostini, `BROWSER` is usually set to
`garcon-url-handler`, which opens URLs in the host OS Chrome
browser. That browser lacks access to the Linux file system.

This commit fixes these commands. `help` now opens the URL on
www.fishshell.com.  `fish_config` now opens the URL for the
server it starts. Previously, it opened a local file that
redirects to the same URL.

In the case of `help`, the situation could be improved further
by starting a web server to serve help. I don't know of another
way to access `/share/fish` from outside the VM without user
intervention, and I think that might be a part of the security
model for the Crostini VM.

It's hard to write a test for this. I checked that `help math`,
`python2 webconfig.py`, and `python3 webconfig.py` work on my
machine running in Crostini.
This isn't really necessary, but it makes the file look nicer to
my eyes. Let me know if you want me to remove this commit.
@ilyagr
Copy link
Contributor Author

ilyagr commented Mar 7, 2021

I realized that the field webbrowser.get().name I used is, strictly speaking, not documented. I checked, it has been around since Python 2.5. I suppose it's not completely inconceivable that it might one day disappear, or that it's not present in PyPy.

I don't know a better way to do this, though. If we are worried enough, we could sacrifice some aesthetics and make sure it never fails:

try:
    return "garcon-url-handler" in webbrowser.get().name
except NameError:
    # Explanation
    return False

@faho faho merged commit fe70c29 into fish-shell:master Mar 7, 2021
@faho
Copy link
Member

faho commented Mar 7, 2021

or that it's not present in PyPy.

Well, that would make PyPy incompatible, which would probably be a bug in PyPy.


Considering that the "name" attribute is on the BaseBrowser class I think it's safe enough.

Merged, thanks!

@faho faho added this to the fish 3.2.1 milestone Mar 7, 2021
@ilyagr
Copy link
Contributor Author

ilyagr commented Mar 7, 2021 via email

@ilyagr ilyagr deleted the chromeos-browser branch March 8, 2021 08:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2021
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

2 participants