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

vim.ui.open: "gx" without netrw #23401

Merged
merged 3 commits into from
Jul 5, 2023
Merged

vim.ui.open: "gx" without netrw #23401

merged 3 commits into from
Jul 5, 2023

Conversation

mrshmllow
Copy link
Contributor

@mrshmllow mrshmllow commented Apr 30, 2023

Fix #23231

vim.ui.os_open is partially based off https://github.com/folke/lazy.nvim/blob/main/lua/lazy/util.lua#L13-L44, as recommended by @folke on the original issue.

  • Implement vim.ui.os_open(uri: string | vim.uri.Uri)
    • Accept a string as uri
    • Accept a vim.uri.Uri as uri
  • Implement "gx" in lua. The implementation drops current netrw options, including g:netrw_gx.
  • Ability to change impl by using vim.func.on_func(vim.ui, 'open_uri', …)

@zeertzjq zeertzjq requested a review from justinmk April 30, 2023 06:33
@mrshmllow mrshmllow force-pushed the open_uri branch 2 times, most recently from 5edb19f to 938e299 Compare April 30, 2023 06:43
runtime/lua/vim/ui.lua Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
@glepnir
Copy link
Member

glepnir commented Apr 30, 2023

the uri need check include some symbols or not , like # . in some lsp hover response the link like file:///usr/local/share/doc/xxx/System-IO.html#v:putStrLn . so you need use vim.fn.escape to escape # in uri .

@clason
Copy link
Member

clason commented Apr 30, 2023

Is deleting code from runtime/plugin/netrwPlugin.vim appropriate / allowed?

No, please leave it as is (for now). It's better to set the corresponding global variables to skip setting up the mapping.

We'll completely remove netrw once we have a full replacement.

What is the correct way to add new built-in keymaps in lua? The best location I found was runtime/plugin/nvim.lua.

Yes, that would be the place. But maybe make it opt-in for now?

@mrshmllow
Copy link
Contributor Author

God, the commit history in this PR is a complete mess lmao

runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
@mrshmllow
Copy link
Contributor Author

No, please leave it as is (for now). It's better to set the corresponding global variables to skip setting up the mapping.

I am confused about the execution order, where would you set a global variable in time? I cannot find a place where this is done

the uri need check include some symbols or not , like # . in some lsp hover response the link like file:///usr/local/share/doc/xxx/System-IO.html#v:putStrLn . so you need use vim.fn.escape to escape # in uri .

Im not entirely sure what you mean. Most applications will not understand what #v:putStrLn means. You can try to maybe adjust g:netrw_gx to capture the full #? (as <cfile> rightfully does not expect a file to have #)

@glepnir
Copy link
Member

glepnir commented Apr 30, 2023

it's a markdown link return by some lsp server. it has a # target symbol . but maybe it should not escape in open_uri function. if I want use open_uri to open it i need escape # symbol by myself :)

@clason clason added lua stdlib netrw fuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu ui labels Apr 30, 2023
@github-actions github-actions bot requested review from bfredl and famiu April 30, 2023 12:41
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
@clason
Copy link
Member

clason commented Apr 30, 2023

I am confused about the execution order, where would you set a global variable in time? I cannot find a place where this is done

In your init.vim. The point is only to add the functionality for now so users can use that instead of netRW if they choose. In a second step (after the early access crowd has found the majority of bugs), we can make this the default.

runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/plugin/nvim.lua Outdated Show resolved Hide resolved
runtime/plugin/nvim.lua Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/doc/news.txt Outdated Show resolved Hide resolved
runtime/doc/news.txt Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
runtime/lua/vim/ui.lua Outdated Show resolved Hide resolved
@mrshmllow mrshmllow changed the title Implement vim.ui.open_uri & "gx" in lua without netrw Implement vim.ui.os_open & "gx" in lua without netrw May 26, 2023
Problem:
Showing an error via vim.notify() makes it awkward for callers such as
lsp/handlers.lua to avoid showing redundant errors.

Solution:
Return the message instead of showing it. Let the caller decide whether
and when to show the message.
@mrshmllow
Copy link
Contributor Author

I do hate explorer but start is somehow worse. currently with explorer on windows vim.ui.open("google.com") will open the file manager at some random location without warning.

But start requires a bunch of weird stuff. start for some reason interprets the first argument as the title, but not always(?). start also is entirely different depending on the shell and has some strangeness with spaces, which explorer properly handles.

@justinmk
Copy link
Member

justinmk commented Jul 5, 2023

currently with explorer on windows vim.ui.open("google.com") will open the file manager at some random location without warning.

without "https://" that would be treated as a filepath on any system, afaik.

If you find that netrw's gx on Windows works better, we could use its start rundll32 url.dll,FileProtocolHandler approach:

call s:NetrwExe('sil! !start rundll32 url.dll,FileProtocolHandler '.s:ShellEscape(fname,1))
elseif executable("rundll32")
call s:NetrwExe('sil! !rundll32 url.dll,FileProtocolHandler '.s:ShellEscape(fname,1))

@justinmk justinmk merged commit 5936a88 into neovim:master Jul 5, 2023
10 of 12 checks passed
@mrshmllow mrshmllow deleted the open_uri branch July 6, 2023 02:59
@lourenci
Copy link

lourenci commented Jul 7, 2023

I should be able to not load netrw plugin completely after this, shouldn't I?

vim.g.loaded_netrwPlugin = 1
vim.g.loaded_netrw = 1

gx errors me with:

E117: Unknown function: netrw#GX
E116: Invalid arguments for function netrw#BrowseX

Thanks.

@zeertzjq
Copy link
Member

zeertzjq commented Jul 7, 2023

Cannot reproduce

@lourenci
Copy link

lourenci commented Jul 7, 2023

Got it. I don't know why, but the netrw plugin share/nvim/runtime/plugin/netrwPlugin.vim is being loaded before the init.luafile (where I tell nvim to not load netrw through the vim.g.loaded_netrwPlugin global var). 🤷🏻‍♂️ Maybe there is some other issue there, or even some misconfiguration on my side.

image

@justinmk
Copy link
Member

justinmk commented Jul 7, 2023

the netrw plugin share/nvim/runtime/plugin/netrwPlugin.vim is being loaded before the init.luafile (where I tell nvim to not load netrw through the vim.g.loaded_netrwPlugin global var)

May need to revert #24262 , or move this mapping to

function vim._init_default_mappings()

@adelarsq
Copy link

adelarsq commented Jul 18, 2023

@justinmk On Windows this call fails:

lua vim.ui.open('https://devdocs.io/#q=lua%20lua_call')

I think that the reason is that Explorer doesn't accept the URL:

image

The message says the its an invalid URL but its a valid one for devdocs.

Is there any alternative to avoid this dependency?

@justinmk
Copy link
Member

justinmk commented Jul 18, 2023

I think that the reason is that Explorer doesn't accept the URL ... Is there any alternative to avoid this dependency?

See above, #23401 (comment) . Patch welcome, having someone with a Windows machine would be helpful.

@mrshmllow
Copy link
Contributor Author

maybe it's worth switching the opener strategy on windows

@justinmk
Copy link
Member

is that different than what i said in #23401 (comment) ?

@zeertzjq zeertzjq added this to the 0.10 milestone Jul 19, 2023
@zeertzjq zeertzjq removed the ui label Jul 19, 2023
@mrshmllow
Copy link
Contributor Author

#24392

@adelarsq
Copy link

is that different than what i said in #23401 (comment) ?

It's the same issue.

#24392

I had confirmed that with this fix vim.ui.open works as expected on Windows.

justinmk pushed a commit to mrshmllow/neovim that referenced this pull request Jul 21, 2023
Problem:
On Windows, `explorer.exe` fails to open some URLs, for example:

    :lua vim.ui.open('https://devdocs.io/#q=lua%20lua_call')

neovim#23401 (comment)

Solution:
Use rundll32 instead.
@mrshmllow
Copy link
Contributor Author

May need to revert #24262 , or move this mapping to...

#24420

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

Successfully merging this pull request may close these issues.

"gx" (open URL in browser) without netrw