gitlinker.nvim icon indicating copy to clipboard operation
gitlinker.nvim copied to clipboard

Provide callbacks as native Vim commands

Open roginfarrer opened this issue 3 years ago • 6 comments

Describe the solution you'd like

I think this plugin would be a lot easier to use if it provided commands, just like vim-fugitive. Not only would it then be much more approachable to create your own keymappings, but it would make it easier to migrate from vim-fugitive (if the commands were the same).

If it were the same, I'd like to see:

  • :GBrowse: open the current file in the browser (in normal mode and visual mode)
  • :GBrowse!: copy the link to the clipboard (I think this comes from vim-rhubarb)

roginfarrer avatar Aug 16 '21 20:08 roginfarrer

I was able to recreate this behavior with the following in my config:

local function copyToClipboard(mode)
	require('gitlinker').get_buf_range_url(
		mode,
		{ action_callback = require('gitlinker.actions').copy_to_clipboard }
	)
end

local function openInBrowser(mode)
	require('gitlinker').get_buf_range_url(
		mode,
		{ action_callback = require('gitlinker.actions').open_in_browser }
	)
end

function _G.gbrowse(range, bang)
	local mode = range > 0 and 'v' or 'n'
	local hasBang = bang == '!'
	if hasBang then
		return copyToClipboard(mode)
	else
		return openInBrowser(mode)
	end
end

vim.cmd([[
	command! -nargs=0 -range -bang GBrowse call v:lua.gbrowse(<range>, '<bang>')
]])

u.nnoremap('<leader>gc', [[:GBrowse!<CR>]])
u.vnoremap('<leader>gc', [[:'<,'>GBrowse!<CR>]])
u.nnoremap('<leader>go', [[:GBrowse<CR>]])
u.vnoremap('<leader>go', [[:'<,'>GBrowse<CR>]])

roginfarrer avatar Aug 25 '21 16:08 roginfarrer

Hi @roginfarrer , great to hear that!

The decision to not provide native vim commands was deliberate. It goes in line with most new lua plugins and their philosophy. The same goes for the plugin not actually doing anything until a setup() like function is called.

I very much like that since it's transparent and forces the user to acknowledge what he is doing.

Yes, mapping to lua functions his currently very cumbersome but I hope that will improve in the near future with neovim's core rapid development.

I would happily accept a PR with your suggestion above somewhere in the README.

I would even be open to providing those commands if and only if the user calls require'gitlinker'.setup(). Those commands should not be called :GBrowse or anything resembling a fugitive command so that it does not create any chaos.

Thanking for your issue and sorry for the late reply. I'm on holidays until the end of the month.

ruifm avatar Aug 25 '21 17:08 ruifm

No worries about the late reply, enjoy your holiday!

The decision to not provide native vim commands was deliberate. It goes in line with most new lua plugins and their philosophy.

While it may be common, I don't entirely agree that it's a standard nor user-friendly. There's a good conversation about this on reddit. I think some popular counter-examples would be Telescope (which uses :Telescope [arg] commands, Neogit (which uses :Neogit), and Trouble.nvim (which uses :Trouble).

I would even be open to providing those commands if and only if the user calls require'gitlinker'.setup()

Would this prevent users from using packer.nvim's ability to lazy-load using the cmd condition? If you're not familiar, this allows the user to configure certain commands related to a plugin, that when called, will require the specified plugin (so it doesn't initialize on neovim starting)

roginfarrer avatar Aug 27 '21 16:08 roginfarrer

I should add, I respect your decision since it's your plugin! Just interested in continuing the conversation.

roginfarrer avatar Aug 27 '21 16:08 roginfarrer

Unrelated but, @roginfarrer does the code you pasted works for a line range if I manually type GBrowse in the command (instead of invoking using keymap)? I tried, it didn't work.

To reproduce:

  1. Select few lines
  2. Do :GBrowse (which would result in :'<,'>GBrowse as we did a visual selection)
  3. Notice that link that has been created has its end line same as the start line

ajitid avatar Feb 10 '22 18:02 ajitid

Actually I think there's a bug with the range option in the plugin. I suggest opening a new issue for it

roginfarrer avatar Feb 13 '22 18:02 roginfarrer