Allow help browser to be chosen independent of $BROWSER environment variable setting #3131

Closed
TieDyedDevil opened this Issue Jun 10, 2016 · 8 comments

Comments

Projects
None yet
3 participants
@TieDyedDevil
Contributor

TieDyedDevil commented Jun 10, 2016

The attached patch allows help.fish to select a web browser independent of the builtin lists of known browsers and the $BROWSER environment variable. This is useful in a graphical enviroment to view fish help in a text-mode browser (e.g. w3m) despite having a graphical web browser (chosen either via $BROWSER or the internal seach logic of help.fish)

The patch causes help.fish to use the value of $fish_help_browser, if set. The value may include command options if desired, e.g. set fish_help_browser w3m -o confirm_qq=0.

When $fish_help_browser is unset, the original behavior still holds.


Fish version: fish, version 2.3.0

Operating system: Fedora 23; dnf install -y fish

Terminal or terminal emulator: st


*** fish-shell/share/functions/help.fish    2016-06-09 14:41:45.609940760 -0700
--- .config/fish/functions/help.fish    2016-06-09 17:39:31.031803670 -0700
***************
*** 2,8 ****

    # Declare variables to set correct scope
    set -l fish_browser
-   set -l fish_browser_bg

    set -l h syntax completion editor job-control todo bugs history killring help
    set h $h color prompt title variables builtin-overview changes expand
--- 2,7 ----
***************
*** 23,80 ****
    # by the help function defined below.
    #
    set -l graphical_browsers htmlview x-www-browser firefox galeon mozilla konqueror epiphany opera netscape rekonq google-chrome chromium-browser
-   set -l text_browsers htmlview www-browser links elinks lynx w3m

!   if type -q "$BROWSER"
!       # User has manually set a preferred browser, so we respect that
!       set fish_browser $BROWSER
! 
!       # If browser is known to be graphical, put into background
!       if contains -- $BROWSER $graphical_browsers
!           set fish_browser_bg 1
!       end
    else
!       # Check for a text-based browser.
!       for i in $text_browsers
!           if type -q -f $i
!               set fish_browser $i
!               break
!           end
!       end
! 
!       # If we are in a graphical environment, check if there is a graphical
!       # browser to use instead.
!       if test "$DISPLAY" -a \( "$XAUTHORITY" = "$HOME/.Xauthority" -o "$XAUTHORITY" = "" \)
!           for i in $graphical_browsers
                if type -q -f $i
                    set fish_browser $i
-                   set fish_browser_bg 1
                    break
                end
            end
-       end
- 
-       # If the OS appears to be Windows (graphical), try to use cygstart
-       if type -q cygstart
-           set fish_browser cygstart
-       # If xdg-open is available, just use that
-       else if type -q xdg-open
-           set fish_browser xdg-open
-       end


!       # On OS X, we go through osascript by default
!       if test (uname) = Darwin
!           if type -q osascript
!               set fish_browser osascript
            end
        end
- 
    end

!   if test -z $fish_browser
        printf (_ '%s: Could not find a web browser.\n') help
!               printf (_ 'Please set the variable $BROWSER to a suitable browser and try again.\n\n')
        return 1
    end

--- 22,81 ----
    # by the help function defined below.
    #
    set -l graphical_browsers htmlview x-www-browser firefox galeon mozilla konqueror epiphany opera netscape rekonq google-chrome chromium-browser

!   if type -q "$fish_help_browser[1]"
!       # User has set fish-specific help browser. This overrides the
!       # browser that may be defined by $BROWSER. The $FISH_HELP_BROWSER
!       # variable may be an array containing a browser name plus options.
!       set fish_browser $fish_help_browser
    else
!       set -l text_browsers htmlview www-browser links elinks lynx w3m
!   
!       if type -q "$BROWSER"
!           # User has manually set a preferred browser, so we respect that
!           set fish_browser $BROWSER
!       else
!           # Check for a text-based browser.
!           for i in $text_browsers
                if type -q -f $i
                    set fish_browser $i
                    break
                end
            end

+           # If we are in a graphical environment, check if there is a graphical
+           # browser to use instead.
+           if test "$DISPLAY" -a \( "$XAUTHORITY" = "$HOME/.Xauthority" -o "$XAUTHORITY" = "" \)
+               for i in $graphical_browsers
+                   if type -q -f $i
+                       set fish_browser $i
+                       break
+                   end
+               end
+           end

!           # If the OS appears to be Windows (graphical), try to use cygstart
!           if type -q cygstart
!               set fish_browser cygstart
!           # If xdg-open is available, just use that
!           else if type -q xdg-open
!               set fish_browser xdg-open
!           end
!       
!       
!           # On OS X, we go through osascript by default
!           if test (uname) = Darwin
!               if type -q osascript
!                   set fish_browser osascript
!               end
            end
+   
        end
    end

!   if test -z $fish_browser[1]
        printf (_ '%s: Could not find a web browser.\n') help
!               printf (_ 'Please set the variable $BROWSER or $fish_help_browser and try again.\n\n')
        return 1
    end

***************
*** 109,114 ****
--- 110,118 ----
                if test -f "$man_arg"
                    man $man_arg
                    return
+               else if test -f "$man_arg.gz"
+                   man (gunzip -c "$man_arg.gz" | psub -s .1)
+                   return
                end
            end
            set fish_help_page "index.html"
***************
*** 136,149 ****
        return
    end

-   if test $fish_browser_bg

!       switch $fish_browser
            case 'htmlview' 'x-www-browser'
                printf (_ 'help: Help is being displayed in your default browser.\n')

            case '*'
!               printf (_ 'help: Help is being displayed in %s.\n') $fish_browser

        end

--- 140,154 ----
        return
    end


!   # If browser is known to be graphical, put into background
!   if contains -- $fish_browser[1] $graphical_browsers
!       switch $fish_browser[1]
            case 'htmlview' 'x-www-browser'
                printf (_ 'help: Help is being displayed in your default browser.\n')

            case '*'
!               printf (_ 'help: Help is being displayed in %s.\n') $fish_browser[1]

        end
@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Jun 10, 2016

Member

This seems like it would be okay to add, but I'm having a hard time reading your diff - which format is that, exactly? I enjoy the "unified" format the most, and it tends to have the best support in e.g. github's tooling.

Though I feel obligated to ask: Why do you want to pick a special browser for help? Usually browsing is nicest in a graphical browser, and when you're running graphical anyway, why not open a tab there?

Member

faho commented Jun 10, 2016

This seems like it would be okay to add, but I'm having a hard time reading your diff - which format is that, exactly? I enjoy the "unified" format the most, and it tends to have the best support in e.g. github's tooling.

Though I feel obligated to ask: Why do you want to pick a special browser for help? Usually browsing is nicest in a graphical browser, and when you're running graphical anyway, why not open a tab there?

@faho faho added the enhancement label Jun 10, 2016

@faho faho added this to the fish-future milestone Jun 10, 2016

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Jun 10, 2016

Member

If you want a text browser, why not simply set BROWSER? Do you actually have that set to something but find it working for fish but causing you annoyance with other software?

I ask as I wonder if you are perhaps working around/against something pretty annoying we do in webconfig.py to prevent CLI/text-based browsers from working properly through python's webbrowser module

(We unset $TERM to intentionally attempt to lock w3m, etc out - this affects the logic of webbrowser.py.)

Member

floam commented Jun 10, 2016

If you want a text browser, why not simply set BROWSER? Do you actually have that set to something but find it working for fish but causing you annoyance with other software?

I ask as I wonder if you are perhaps working around/against something pretty annoying we do in webconfig.py to prevent CLI/text-based browsers from working properly through python's webbrowser module

(We unset $TERM to intentionally attempt to lock w3m, etc out - this affects the logic of webbrowser.py.)

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Jun 10, 2016

Member

Specifically, I wonder if what was being chalked up to as the "the internal seach logic of help.fish" in the comment here was actually the unkind search logic of the python script - and perhaps just by removing the highlighted lines on the python script if it'd be doing what you want automatically?

Member

floam commented Jun 10, 2016

Specifically, I wonder if what was being chalked up to as the "the internal seach logic of help.fish" in the comment here was actually the unkind search logic of the python script - and perhaps just by removing the highlighted lines on the python script if it'd be doing what you want automatically?

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Jun 10, 2016

Member

Blah! Help and config both launch browsers and both have a .fish function, but of course they are not the same thing! Never mind.

Member

floam commented Jun 10, 2016

Blah! Help and config both launch browsers and both have a .fish function, but of course they are not the same thing! Never mind.

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Jun 10, 2016

Member

That patch above is a context diff - supposedly easier to read the resulting code, but for me... context.. is lost if I don't know what the code looked like before -- his patch works fine and applies if you use patch, and after that git diff is happy to show me what it did:

diff --git a/share/functions/help.fish b/share/functions/help.fish
index 23467a4..c13a972 100644
--- a/share/functions/help.fish
+++ b/share/functions/help.fish
@@ -2,7 +2,6 @@ function help --description 'Show help for the fish shell'

    # Declare variables to set correct scope
    set -l fish_browser
-   set -l fish_browser_bg

    set -l h syntax completion editor job-control todo bugs history killring help
    set h $h color prompt title variables builtin-overview changes expand
@@ -23,58 +22,60 @@ function help --description 'Show help for the fish shell'
    # by the help function defined below.
    #
    set -l graphical_browsers htmlview x-www-browser firefox galeon mozilla konqueror epiphany opera netscape rekonq google-chrome chromium-browser
-   set -l text_browsers htmlview www-browser links elinks lynx w3m

-   if type -q "$BROWSER"
-       # User has manually set a preferred browser, so we respect that
-       set fish_browser $BROWSER
-
-       # If browser is known to be graphical, put into background
-       if contains -- $BROWSER $graphical_browsers
-           set fish_browser_bg 1
-       end
+   if type -q "$fish_help_browser[1]"
+       # User has set fish-specific help browser. This overrides the
+       # browser that may be defined by $BROWSER. The $FISH_HELP_BROWSER
+       # variable may be an array containing a browser name plus options.
+       set fish_browser $fish_help_browser
    else
-       # Check for a text-based browser.
-       for i in $text_browsers
-           if type -q -f $i
-               set fish_browser $i
-               break
-           end
-       end
-
-       # If we are in a graphical environment, check if there is a graphical
-       # browser to use instead.
-       if test "$DISPLAY" -a \( "$XAUTHORITY" = "$HOME/.Xauthority" -o "$XAUTHORITY" = "" \)
-           for i in $graphical_browsers
+       set -l text_browsers htmlview www-browser links elinks lynx w3m
+   
+       if type -q "$BROWSER"
+           # User has manually set a preferred browser, so we respect that
+           set fish_browser $BROWSER
+       else
+           # Check for a text-based browser.
+           for i in $text_browsers
                if type -q -f $i
                    set fish_browser $i
-                   set fish_browser_bg 1
                    break
                end
            end
-       end
-
-       # If the OS appears to be Windows (graphical), try to use cygstart
-       if type -q cygstart
-           set fish_browser cygstart
-       # If xdg-open is available, just use that
-       else if type -q xdg-open
-           set fish_browser xdg-open
-       end

+           # If we are in a graphical environment, check if there is a graphical
+           # browser to use instead.
+           if test "$DISPLAY" -a \( "$XAUTHORITY" = "$HOME/.Xauthority" -o "$XAUTHORITY" = "" \)
+               for i in $graphical_browsers
+                   if type -q -f $i
+                       set fish_browser $i
+                       break
+                   end
+               end
+           end

-       # On OS X, we go through osascript by default
-       if test (uname) = Darwin
-           if type -q osascript
-               set fish_browser osascript
+           # If the OS appears to be Windows (graphical), try to use cygstart
+           if type -q cygstart
+               set fish_browser cygstart
+           # If xdg-open is available, just use that
+           else if type -q xdg-open
+               set fish_browser xdg-open
+           end
+       
+       
+           # On OS X, we go through osascript by default
+           if test (uname) = Darwin
+               if type -q osascript
+                   set fish_browser osascript
+               end
            end
+   
        end
-
    end

-   if test -z $fish_browser
+   if test -z $fish_browser[1]
        printf (_ '%s: Could not find a web browser.\n') help
-               printf (_ 'Please set the variable $BROWSER to a suitable browser and try again.\n\n')
+               printf (_ 'Please set the variable $BROWSER or $fish_help_browser and try again.\n\n')
        return 1
    end

@@ -109,6 +110,9 @@ function help --description 'Show help for the fish shell'
                if test -f "$man_arg"
                    man $man_arg
                    return
+               else if test -f "$man_arg.gz"
+                   man (gunzip -c "$man_arg.gz" | psub -s .1)
+                   return
                end
            end
            set fish_help_page "index.html"
@@ -136,14 +140,15 @@ function help --description 'Show help for the fish shell'
        return
    end

-   if test $fish_browser_bg

-       switch $fish_browser
+   # If browser is known to be graphical, put into background
+   if contains -- $fish_browser[1] $graphical_browsers
+       switch $fish_browser[1]
            case 'htmlview' 'x-www-browser'
                printf (_ 'help: Help is being displayed in your default browser.\n')

            case '*'
-               printf (_ 'help: Help is being displayed in %s.\n') $fish_browser
+               printf (_ 'help: Help is being displayed in %s.\n') $fish_browser[1]

        end
Member

floam commented Jun 10, 2016

That patch above is a context diff - supposedly easier to read the resulting code, but for me... context.. is lost if I don't know what the code looked like before -- his patch works fine and applies if you use patch, and after that git diff is happy to show me what it did:

diff --git a/share/functions/help.fish b/share/functions/help.fish
index 23467a4..c13a972 100644
--- a/share/functions/help.fish
+++ b/share/functions/help.fish
@@ -2,7 +2,6 @@ function help --description 'Show help for the fish shell'

    # Declare variables to set correct scope
    set -l fish_browser
-   set -l fish_browser_bg

    set -l h syntax completion editor job-control todo bugs history killring help
    set h $h color prompt title variables builtin-overview changes expand
@@ -23,58 +22,60 @@ function help --description 'Show help for the fish shell'
    # by the help function defined below.
    #
    set -l graphical_browsers htmlview x-www-browser firefox galeon mozilla konqueror epiphany opera netscape rekonq google-chrome chromium-browser
-   set -l text_browsers htmlview www-browser links elinks lynx w3m

-   if type -q "$BROWSER"
-       # User has manually set a preferred browser, so we respect that
-       set fish_browser $BROWSER
-
-       # If browser is known to be graphical, put into background
-       if contains -- $BROWSER $graphical_browsers
-           set fish_browser_bg 1
-       end
+   if type -q "$fish_help_browser[1]"
+       # User has set fish-specific help browser. This overrides the
+       # browser that may be defined by $BROWSER. The $FISH_HELP_BROWSER
+       # variable may be an array containing a browser name plus options.
+       set fish_browser $fish_help_browser
    else
-       # Check for a text-based browser.
-       for i in $text_browsers
-           if type -q -f $i
-               set fish_browser $i
-               break
-           end
-       end
-
-       # If we are in a graphical environment, check if there is a graphical
-       # browser to use instead.
-       if test "$DISPLAY" -a \( "$XAUTHORITY" = "$HOME/.Xauthority" -o "$XAUTHORITY" = "" \)
-           for i in $graphical_browsers
+       set -l text_browsers htmlview www-browser links elinks lynx w3m
+   
+       if type -q "$BROWSER"
+           # User has manually set a preferred browser, so we respect that
+           set fish_browser $BROWSER
+       else
+           # Check for a text-based browser.
+           for i in $text_browsers
                if type -q -f $i
                    set fish_browser $i
-                   set fish_browser_bg 1
                    break
                end
            end
-       end
-
-       # If the OS appears to be Windows (graphical), try to use cygstart
-       if type -q cygstart
-           set fish_browser cygstart
-       # If xdg-open is available, just use that
-       else if type -q xdg-open
-           set fish_browser xdg-open
-       end

+           # If we are in a graphical environment, check if there is a graphical
+           # browser to use instead.
+           if test "$DISPLAY" -a \( "$XAUTHORITY" = "$HOME/.Xauthority" -o "$XAUTHORITY" = "" \)
+               for i in $graphical_browsers
+                   if type -q -f $i
+                       set fish_browser $i
+                       break
+                   end
+               end
+           end

-       # On OS X, we go through osascript by default
-       if test (uname) = Darwin
-           if type -q osascript
-               set fish_browser osascript
+           # If the OS appears to be Windows (graphical), try to use cygstart
+           if type -q cygstart
+               set fish_browser cygstart
+           # If xdg-open is available, just use that
+           else if type -q xdg-open
+               set fish_browser xdg-open
+           end
+       
+       
+           # On OS X, we go through osascript by default
+           if test (uname) = Darwin
+               if type -q osascript
+                   set fish_browser osascript
+               end
            end
+   
        end
-
    end

-   if test -z $fish_browser
+   if test -z $fish_browser[1]
        printf (_ '%s: Could not find a web browser.\n') help
-               printf (_ 'Please set the variable $BROWSER to a suitable browser and try again.\n\n')
+               printf (_ 'Please set the variable $BROWSER or $fish_help_browser and try again.\n\n')
        return 1
    end

@@ -109,6 +110,9 @@ function help --description 'Show help for the fish shell'
                if test -f "$man_arg"
                    man $man_arg
                    return
+               else if test -f "$man_arg.gz"
+                   man (gunzip -c "$man_arg.gz" | psub -s .1)
+                   return
                end
            end
            set fish_help_page "index.html"
@@ -136,14 +140,15 @@ function help --description 'Show help for the fish shell'
        return
    end

-   if test $fish_browser_bg

-       switch $fish_browser
+   # If browser is known to be graphical, put into background
+   if contains -- $fish_browser[1] $graphical_browsers
+       switch $fish_browser[1]
            case 'htmlview' 'x-www-browser'
                printf (_ 'help: Help is being displayed in your default browser.\n')

            case '*'
-               printf (_ 'help: Help is being displayed in %s.\n') $fish_browser
+               printf (_ 'help: Help is being displayed in %s.\n') $fish_browser[1]

        end
@TieDyedDevil

This comment has been minimized.

Show comment
Hide comment
@TieDyedDevil

TieDyedDevil Jun 10, 2016

Contributor

To answer the "why?" question:

I run a tiling wm. My "graphical" web browser (qutebrowser), which is referenced by $BROWSER, is almost fully keyboard-navigable. I try to avoid the use of a pointer device as much as possible, having learned over the years that my RSI flares up with use of a pointer device.

While qutebrowser is appropriate for a lot of tasks, there are some things it doesn't do well. In particular, there's no good way to set keyboard focus for scrolling when a page has a scrollable div (as do the fish pages). Also, qutebrowser's handling of multiple instances is less than ideal for my use case. (I've tried pretty much all of the keyboard-navigable browsers; qutebrowser presents the fewest annoyances.)

I like w3m for reading text-heavy pages. Also, there's no visual "context shift" needed when popping up a w3m instance; it opens in the current terminal pane.

So despite having qutebrowser as my main browser, I prefer to use w3m for fish help. (I don't use the browser to configure fish.)

I hope this clarifies my intent.

(Also: sorry about the context diff. Point taken.)

Contributor

TieDyedDevil commented Jun 10, 2016

To answer the "why?" question:

I run a tiling wm. My "graphical" web browser (qutebrowser), which is referenced by $BROWSER, is almost fully keyboard-navigable. I try to avoid the use of a pointer device as much as possible, having learned over the years that my RSI flares up with use of a pointer device.

While qutebrowser is appropriate for a lot of tasks, there are some things it doesn't do well. In particular, there's no good way to set keyboard focus for scrolling when a page has a scrollable div (as do the fish pages). Also, qutebrowser's handling of multiple instances is less than ideal for my use case. (I've tried pretty much all of the keyboard-navigable browsers; qutebrowser presents the fewest annoyances.)

I like w3m for reading text-heavy pages. Also, there's no visual "context shift" needed when popping up a w3m instance; it opens in the current terminal pane.

So despite having qutebrowser as my main browser, I prefer to use w3m for fish help. (I don't use the browser to configure fish.)

I hope this clarifies my intent.

(Also: sorry about the context diff. Point taken.)

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Jun 18, 2016

Member

I see that this one got an easy-pick label added (an invitation/suggestion to anyone wanting to help that this would be a not-too-difficult way to do it). Perhaps submit a pull request @TieDyedDevil if you have the time!

Member

floam commented Jun 18, 2016

I see that this one got an easy-pick label added (an invitation/suggestion to anyone wanting to help that this would be a not-too-difficult way to do it). Perhaps submit a pull request @TieDyedDevil if you have the time!

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Dec 10, 2016

Member

Closed with merge of #3583 (commit 5ec9fcd).

Member

faho commented Dec 10, 2016

Closed with merge of #3583 (commit 5ec9fcd).

@faho faho closed this Dec 10, 2016

@faho faho added the release notes label Dec 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment