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

support open file and link on WSL #1997

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

shinhs0506
Copy link
Contributor

fixes #1996

)

func isWSL() bool {
data, err := ioutil.ReadFile("/proc/sys/kernel/osrelease")
Copy link
Owner

Choose a reason for hiding this comment

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

@shinhs0506 is this a large file? I wanna get an idea of the cost of reading it at startup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The file is one line long

just as a reference, microsoft/WSL#423 (comment)

my /proc/sys/kernel/osrelease has the following content
5.10.102.1-microsoft-standard-WSL2

Copy link
Owner

Choose a reason for hiding this comment

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

sounds good!

Copy link
Owner

@jesseduffield jesseduffield left a comment

Choose a reason for hiding this comment

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

Just one question :) marking as changes requested for my own bookkeeping

@jesseduffield
Copy link
Owner

@shinhs0506 looks like you've gotta run gofumpt on pkg/config/config_linux.go (see https://github.com/mvdan/gofumpt)

@jesseduffield jesseduffield merged commit 582b199 into jesseduffield:master Jul 4, 2022
@jesseduffield
Copy link
Owner

Nice work @shinhs0506 !

@shinhs0506
Copy link
Contributor Author

should also fix #1513

@pirafrank
Copy link

pirafrank commented Sep 3, 2024

Sorry to add to this, but I think there should be proper handling of setups where xdg-open is not installed, or a browser is not available in the terminal session lazygit runs in.

The error below appears on WSL2 using wezterm and (ctrl) clicking on links to try opening them in the default browser. lazygit crashes because of xdg-open: command not found (lazygit 0.40.2 on WSL2 Ubuntu 22.04 on Windows 11).

2024/09/03 12:33:31 An error occurred! Please create an issue at: https://github.com/jesseduffield/lazygit/issues

*errors.errorString bash: line 1: xdg-open: command not found

/home/runner/work/lazygit/lazygit/pkg/commands/oscommands/cmd_obj_runner.go:200 (0x9777db)
/home/runner/work/lazygit/lazygit/pkg/commands/oscommands/cmd_obj_runner.go:107 (0x97682c)
/home/runner/work/lazygit/lazygit/pkg/commands/oscommands/cmd_obj_runner.go:45 (0x975ea5)
/home/runner/work/lazygit/lazygit/pkg/commands/oscommands/cmd_obj.go:190 (0x97506f)
/home/runner/work/lazygit/lazygit/pkg/commands/oscommands/os.go:110 (0x979e77)
/home/runner/work/lazygit/lazygit/pkg/gui/information_panel.go:44 (0xaf41c6)
/home/runner/work/lazygit/lazygit/pkg/gui/keybindings.go:387 (0xaf810f)
/home/runner/work/lazygit/lazygit/pkg/gui/keybindings.go:373 (0xaf803b)
/home/runner/work/lazygit/lazygit/vendor/github.com/jesseduffield/gocui/gui.go:1424 (0x760fd9)
/home/runner/work/lazygit/lazygit/vendor/github.com/jesseduffield/gocui/gui.go:1392 (0x760e71)
/home/runner/work/lazygit/lazygit/vendor/github.com/jesseduffield/gocui/gui.go:1300 (0x760671)
/home/runner/work/lazygit/lazygit/vendor/github.com/jesseduffield/gocui/gui.go:798 (0x75ea0a)
/home/runner/work/lazygit/lazygit/vendor/github.com/jesseduffield/gocui/gui.go:752 (0x75e58b)
/home/runner/work/lazygit/lazygit/vendor/github.com/jesseduffield/gocui/gui.go:739 (0x75e2a5)
/home/runner/work/lazygit/lazygit/pkg/gui/gui.go:669 (0xaf0885)
/home/runner/work/lazygit/lazygit/pkg/gui/gui.go:675 (0xaf0dac)
/home/runner/work/lazygit/lazygit/pkg/utils/utils.go:108 (0x82b2c7)
/home/runner/work/lazygit/lazygit/pkg/gui/gui.go:674 (0xaf0cf4)
/home/runner/work/lazygit/lazygit/pkg/app/app.go:263 (0xb1afe9)
/home/runner/work/lazygit/lazygit/pkg/app/app.go:48 (0xb1af7e)
/home/runner/work/lazygit/lazygit/pkg/app/entry_point.go:150 (0xb1d286)
/home/runner/work/lazygit/lazygit/main.go:23 (0xb1eb1e)
/opt/hostedtoolcache/go/1.20.7/x64/src/runtime/internal/atomic/types.go:194 (0x437c87)
/opt/hostedtoolcache/go/1.20.7/x64/src/runtime/asm_amd64.s:1598 (0x467ec1)

A solution for this case may be to fallback to x-www-browser or do something similar to gh browse of GitHub CLI, which works in WSL2 and opens links in the default browser on Windows.

A few additions: it seems that this control somehow does not work. Also, powershell.exe may not be in $PATH because it has been disabled by the user (most common case Win filesystem integration having slow reads and slowing the interactive WSL shell experience).

Edited to add info about code.

@LoliPain
Copy link

@pirafrank #1997 (comment)

Also, powershell.exe may not be in $PATH because it has been disabled by the user

sudo ln -s /mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe /usr/local/bin/powershell.exe

@LoliPain
Copy link

But generally, I agree that this PR is nonsense and should be reversed.

The true motivation of the author of this PR is apparently clear only to himself, in which case he can set himself a fork with this change instead of making these destructive changes in the master
Also, I found THIS which makes it even more hilarious reading about

It is not recommended to use xdg-open as root. I feel like I would need to install this package as root to use with WSL Unbuntu. Isn't this dangerous?

Even tho xdg-open runs as common binary from the user you started it from, no matter how it was installed (I never would have thought that I would have to explain this to anyone)

❯ whoami
loli
❯ xdg-open . | ps -aux | grep xdg-open
loli     14921  0.0  0.0   2616  1748 pts/9    S+   06:18   0:00 /bin/sh /usr/bin/xdg-open .

To sum up: PR has done more harm than good, at least because most of everyone I know uses xdg-open along with WSL2, and now at least @pirafrank and me should have to use fix for this "fix"

At least those who vitally need to use open via powershell start can specify this in ~/.config/lazygit/config.yml, so as to leave the most common scenario of working with xdg-open enabled by default

@jesseduffield Would you and you be so kind as to consider this matter?

@pirafrank
Copy link

I agree with the comment above. Also, I think this pkg may help https://github.com/cli/browser.

Customizing the config.yml is last resort imo, first because I think this should be handled in code, and second because a lot of people have their dotfiles repo shared cross-platform.

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

Successfully merging this pull request may close these issues.

open file and open link returns error in WSL
4 participants