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

Regression of rustup doc BROWSER environment variable support in #1287 #2642

Closed
jer-irl opened this issue Jan 21, 2021 · 11 comments
Closed

Regression of rustup doc BROWSER environment variable support in #1287 #2642

jer-irl opened this issue Jan 21, 2021 · 11 comments
Labels
Milestone

Comments

@jer-irl
Copy link

jer-irl commented Jan 21, 2021

Problem

Bug #1287 and PR #1289 added support for the BROWSER environment variable to allow overrides to the XDG default browser when running rustup doc.

Steps

  1. BROWSER=$(some-non-default) rustup doc
  2. Documentation opened in XDG default browser

Possible Solution(s)

It looks to me like this feature was reverted with the move to opener in this commit e661fc6

It looks like cargo still handles this environment variable and just falls back to opener https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/cargo_doc.rs#L101 . Would PRs to restore this feature be welcome? I'd imagine it would be pretty small and self-contained.

Notes

Output of rustup --version: cargo 1.49.0 (d00d64df9 2020-12-05)
Output of rustup show:

Default host: x86_64-unknown-linux-gnu
rustup home:  /home/jeremy/.rustup

stable-x86_64-unknown-linux-gnu (default)
rustc 1.49.0 (e1884a8e3 2020-12-29)
@jer-irl jer-irl added the bug label Jan 21, 2021
@kinnison
Copy link
Contributor

While I concur that this is a regression, I wonder if the right answer is to push the handling of BROWSER into the opener crate, rather than duplicating logic between here and Cargo.

@Seeker14491 how would you feel about BROWSER environment variable handling being pushed into opener as per the linked approach in Cargo?

@Seeker14491
Copy link
Contributor

Having the BROWSER override implemented in opener is planned for the next release and is already implemented in the v0.5-dev branch.

@kinnison
Copy link
Contributor

@Seeker14491 That's fantastic news. Do you have a timeline on releasing that?

@kinnison
Copy link
Contributor

kinnison commented Jun 8, 2021

@Seeker14491 I appreciate we're pinging you quite a bit recently, but... any word on 0.5 ?

@Seeker14491
Copy link
Contributor

I'm having another go at fixing several issues people have reported in the opener crate, and I plan on putting out a release within a few days at most.

@kinnison
Copy link
Contributor

I'm having another go at fixing several issues people have reported in the opener crate, and I plan on putting out a release within a few days at most.

That is fantastic news. Please don't let me rush you though - take your time :D

@kinnison kinnison added this to the 1.25.0 milestone Jun 10, 2021
Seeker14491 added a commit to Seeker14491/rustup.rs that referenced this issue Jun 13, 2021
@Seeker14491
Copy link
Contributor

This should be fixed now with the merging of #2792.

@kinnison
Copy link
Contributor

Aha thanks @Seeker14491

rbtcollins pushed a commit to rbtcollins/rustup.rs that referenced this issue Jul 11, 2021
@sudoforge
Copy link

sudoforge commented Aug 21, 2021

Is there any chance that this could be added to a 1.24.4 release?

@sudoforge
Copy link

sudoforge commented Aug 21, 2021

What I'm really asking/saying is:

I'm impatient and would like this to be considered a hotfix, considering it is a regression.

For additonal context, it doesn't appear to use either the BROWSER environment variable, or the default browser set with XDG. I'm going to open an issue with opener directly.

➜ echo $BROWSER
qutebrowser
                                                                                                                        

➜ xdg-settings get default-web-browser
org.qutebrowser.qutebrowser.desktop


➜ pstree 918976
rustup───sh───chromium─┬─chromium───chromium───19*[{chromium}]
                       ├─chromium───chromium─┬─chromium───5*[{chromium}]
                       │                     └─2*[chromium───12*[{chromium}]]
                       ├─chromium───7*[{chromium}]
                       └─19*[{chromium}]

@sudoforge
Copy link

To any future readers, this was due to the xdg-mime configuration for text/html being set to chromium.desktop. To resolve this:

xdg-mime default <your preferred application> text/html

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

No branches or pull requests

4 participants