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

set mappings = nil could not disable the kemaps #48

Open
meijieru opened this issue Jan 9, 2022 · 11 comments
Open

set mappings = nil could not disable the kemaps #48

meijieru opened this issue Jan 9, 2022 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@meijieru
Copy link

meijieru commented Jan 9, 2022

Describe the bug
As the title, here overrides the mappings.

Here is my config

  require("gitlinker").setup {
    callbacks = {
      ["direct.meijieru.com"] = function(url_data)
        url_data.host = "gitea.meijieru.com"
        return require("gitlinker.hosts").get_gitea_type_url(url_data)
      end,
    },
    mappings = nil,
  }
@meijieru meijieru added the bug Something isn't working label Jan 9, 2022
@ajitid
Copy link

ajitid commented Feb 10, 2022

Looks like resolving this would result in a breaking API change. If nil is fixed, we would have to remove the default mapping (which may or may not be desirable).

mappings = mappings or "<leader>gy"

@meijieru
Copy link
Author

Maybe as a command and let the user map them.

@ajitid
Copy link

ajitid commented Feb 16, 2022

Or use mappings as false to remove mappings but nil (which equates to not defining a mapping through options) to use the default one?

Edit: This is something many plugins do, here's Telescope as an example:

image

@ruifm
Copy link
Owner

ruifm commented Feb 16, 2022

Or use mappings as false to remove mappings but nil (which equates to not defining a mapping through options) to use the default one?

Sounds like the best option.

I think it was a mistake to auto define mappings. I did it to improve the out-of-the-box experience but now I don't think it's such a good idea and whatever is done will definitely break the API 😢

@ajitid
Copy link

ajitid commented Feb 17, 2022

whatever is done will definitely break the API

Kind of. If nil wasn't working for them, and nobody cared to raise an issue about it, it means people rarely considered nullifying gitlinker mappings. So not a huge user base would be affected.

Would you mind taking a PR for this issue?


Unrelated. but in one of the issue I became aware that one cannot give a range through command prompt as the plugin doesn't relies on < and > marks. Was this intended or should I create an issue?

@linrongbin16
Copy link

Submit #77, use "" (empty string) to disable default mappings.

@linrongbin16
Copy link

linrongbin16 commented Feb 19, 2023

Hi @meijieru , @ajitid ,
I have fixed this issue in my fork with PR: linrongbin16/gitlinker.nvim#1
Feel free to use it.

@ajitid
Copy link

ajitid commented Feb 19, 2023

I've already forked and am using a different method to suppress mappings.
Thanks for mentioning it anyway!

@linrongbin16
Copy link

linrongbin16 commented Feb 19, 2023

Cool, I have also fixed another issue #75 (open browser in windows).
I use cmd /C start {url}. this method is copied from: https://github.com/axieax/urlview.nvim/blob/main/lua/urlview/actions.lua#L38.

@ArchitBhonsle
Copy link

So what's the status for this? Noticed that I still had the default mappings even though I set mappings = nil

@linrongbin16
Copy link

linrongbin16 commented Jun 15, 2023

Hi all, I have make my own fork: https://github.com/linrongbin16/gitlinker.nvim.

It fixed this issue, and also support Windows, and refactored a lot of things, I believe it's much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants