ruifm / gitlinker.nvim

A lua neovim plugin to generate shareable file permalinks (with line ranges) for several git web frontend hosts. Inspired by tpope/vim-fugitive's :GBrowse
GNU General Public License v3.0
535 stars 45 forks source link

Provide callbacks as native Vim commands #29

Open roginfarrer opened 3 years ago

roginfarrer commented 3 years ago

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:

roginfarrer commented 3 years ago

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>]])
ruifm commented 3 years ago

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.

roginfarrer commented 3 years ago

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 commented 3 years ago

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

ajitid commented 2 years ago

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
roginfarrer commented 2 years ago

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