Add snap.map/snap.maps with config API and command registration

[camspiers]

The following is my proposed strategy for making registering maps and commands a nicer experience in snap, as well as providing an easy way to create snap-invoking functions with default configuration and a configuration API.

See config/init.fnl for documentation.

The way that you use it with via the new snap.map/snap.maps APIs, which is a less raw version of the snap.register.map API (it uses snap.register.map under the hood.

For example here is some snap map and command registering with the new API:

local file = config.file:with {reverse = true, suffix = "»"}
local vimgrep = config.vimgrep:with {limit = 50000}
local args = {"--hidden", "--iglob", "!.git/*"}

snap.maps {
  {"<Leader><Leader>", file {producer = "ripgrep.file", args = args}, "files"},
  {"<Leader>sssss", file {producer = "ripgrep.file", prompt = "CustomPrompt"}},
  {"<Leader>fg", file {producer = "git.file"}, "git.files"},
  {"<Leader>fb", file {producer = "vim.buffer"}, "buffers"},
  {"<Leader>ff", vimgrep {}, "grep"},
  {"<Leader>fo", file {producer = "vim.oldfile"}, "oldfiles"},
  {"<Leader>fs", file {args = args, try = {"git.file", "ripgrep.file"}}, "git-with-fallback"},
  {"<Leader>aaaa", file {combine = {"vim.buffer", "vim.oldfiles"}}}
}

This will register the specified maps in normal mode, and will also register the functions produced by the defaults.* calls to be run from named commands, e.g. :Snap git.files, or :Snap files.

If possible I would like to get the thoughts/review of @ahmedelgabri @leiserfg @beauwilliams @astridlyre @scwfri @akinsho @babygau @gegoune @zetashift, if you are unable to no worries, I just don't want to land this new API without feedback.

Thanks!!

Screenshot showing the commands:

[camspiers]

Another thing to mention is that there is an API in there for specifying defaults config too:

-- This will create a function equivalent to `defaults.file` but with your custom config merged into each call
local file = defaults.file:with {layout = myCustomLayoutFunction}

-- So the following will now actually be `snap.run {layout = myCustomLayoutFunction, and more configuration}`
snap.map("<Leader><Leader>", file {}, "files")

In your own configs, you might like some defaults like reverse and a particular layout just create your own version of defaults.file and defaults.vimgrep by using the defaults.with API.

[bangedorrunt]

@camspiers, your idea is great, and based on that, here is my opinion. I don't like defaults keyword, and the config seems a bit complicated to me. Moreover, I don't see the way to use try or combine in your example. With snap, you already introduced some concepts of functional programming pattern such as producer/consumer, try and combine... What do you think if you could make use of reducer for snap?

I will split snap functions into 2 categories: built-in and custom.

• Built-in functions are functions implemented by snap such as git_files, files, oldfiles, ripgrep...
• Custom functions are functions which make use of combine/try

A reducer would be in the form (state, config) -> state

• state is the stateless settings that snap.run() uses behind the scene. Hence, with this idea, we only expose snap.create() to user. Please also note that state already defines some defaults

• config has a form of

{
  type = 'git.files' --  built-in functions such as 'files', 'oldfiles', 'ripgrep'...
                    --  or 'custom'
  keymap = '<Leader><Leader>' -- or whatever your map is in normal mode
  reverse,
  layout,
  producer,
  consumer
  select,
  multiselect,
  views,
  ......
}

The following is the pseudo code example of how I would config snap:

-- DYO configs
local git_files_conf = {
  type = 'git.files',
  -- Override configs
  keymap = '<Leader><Leader',
  reverse = true,
  consumer = true
}

local ripgrep_conf = {
  type = 'ripgrep.files',
  -- Override configs
  keymap = '<Leader>;',
  reverse = true,
  limit = 10000
}

local project_conf = {
  type = 'custom',
  -- Override configs
  keymap = '<Leader>p',
  reverse = false,
  comp = 'try' -- or `combine`
  producers = { 'git.files', 'files' },
  consumer = 'fzf',
}
-- Finally, activate snap
snap.create(git_files_conf, ripgrep_conf, project_conf)

IMO, the above example looks clean and easier to understand for new users

Please also note that, I introduce some new keywords such as type, keymap, comp

• Because snap will manage only one global state so type will help snap.run() know which built-in or custom settings need to be overriden
• With keymap, we don't have to expose snap.register()
• comp let you define custom try or combine functions.

Because I'm very new to developer world, this is just my humble opinion. Thank you for making this great plugin.

Edit: This is not purely my idea. I come across this thanks to learn React Redux

[akinsho]

@camspiers just tried this out and will try to give more feedback over the course of the day. So far I really like the simplification of the API this brings. It feels like a lot less to deal with to get started.

A few things I've noted so far,

• I think as mentioned above users lose the ability to use try and combine, maybe this could be done allowing producer to be a list or table of producers to try 🤷🏾 e.g.

  defaults.file { producer = fzf {"git.file", "ripgrep.file"} }

  and under the hood this applies try or something, just a random idea.

Secondly I've noticed that the hidden flag doesn't work only passing it as an argument seems to work (could be a misconfiguration error)

        snap.map(
          "<Leader>ff",
          defaults.file {
            producer = "ripgrep.file",
            consumer = "fzy",
            -- hidden = true,
            args = { "--hidden", "--iglob", "!.git/*" },
          },
          "files"
        )
        snap.map(
          "<Leader>fs",
          defaults.vimgrep {
            limit = 50000,
            -- hidden = true, --< doesn't work
            args = { "--hidden", "--iglob", "!.git/*" }, -- < works
          },
          "grep"
        )

[camspiers]

@babygau While I understand the desire to introduce state like that, as explained in other comments I don't want snap.run or really any part of snap to contain state, as I think long term it will create complexity that is undesirable in that it makes things hard to debug, it makes it hard for people to write plugins to snap that aren't interfered with by someones personal snap state configuration. Interoperability is advanced when every single time you call snap.run with the same function it results in the same output, that is unaffected by global state (excluding the files system and open buffers etc).

One must understand with the API I illustrated above that I am doing nothing but configuring key mappings with an associated function that calls snap.run with a particular config, all the defaults APIs are doing is constructing those functions in a more developer friendly way, but I need some iteration (e.g. the try and combine comments). This doesn't replace any other way of constructing the functions that call snap.run, so for example the snap.create method works fine, as does everyone else's existing configs that use snap.register.map. Hence @akinsho we haven't lost the ability to use things like try and combine, or any other combination of producers and consumers that already exist, as you can still use snap in the exact same way as before, register an arbitrary function that calls snap.run when invoked.

@babygau I have used redux in React, but I am not sure what exactly your proposed introduction of reducer is doing.

I agree with both of your points that we need an ability to more easily use try and combine, however it should be noted that you can pass any function into snap.map, and so all the methods of creating a function yourself that calls snap.run still work, this new API is just a method for creating such functions.

I agree with your point @babygau that we should introducer an ability to create multiple mappings in a single call, but for that I think we can just use something like snap.maps.

@akinsho I think you are probably running into an issue where the hidden flag is incompatible with the args flag. That isn't good, but we could validate against that.

Regarding the defaults name, I'm definitely flexible, what do you propose @babygau ?

I will spend some more time iterating and incorporating your comments, thanks for all the feedback!

[camspiers]

How about:

local file = defaults.file:with {reverse = true}
local vimgrep = defaults.vimgrep:with {limit = 50000}
local args = {"--hidden", "--iglob", "!.git/*"}

snap.maps {
  {"<Leader><Leader>", file {producer = "ripgrep.file", args = args}, "files"},
  {"<Leader>fg", file {producer = "git.file"}, "git.files"},
  {"<Leader>fb", file {producer = "vim.buffer"}, "buffers"},
  {"<Leader>ff", vimgrep {}, "grep"},
  {"<Leader>fo", file {producer = "vim.oldfile"}, "oldfiles"},
  {"<Leader>fs", file {args = args, try = {"git.file", "ripgrep.file"}}, "git-with-fallback"}
}

[camspiers]

I'm thinking I will actually make it so you have to pass the producer flag, that is it doesn't default to ripgrep.file

[camspiers]

Okay I have updated the PR you can now do:

local file = defaults.file:with {reverse = true, suffix = "»"}
local vimgrep = defaults.vimgrep:with {limit = 50000}
local args = {"--hidden", "--iglob", "!.git/*"}

snap.maps {
  {"<Leader><Leader>", file {producer = "ripgrep.file", args = args}, "files"},
  {"<Leader>sssss", file {producer = "ripgrep.file", prompt = "CustomPrompt"}},
  {"<Leader>fg", file {producer = "git.file"}, "git.files"},
  {"<Leader>fb", file {producer = "vim.buffer"}, "buffers"},
  {"<Leader>ff", vimgrep {}, "grep"},
  {"<Leader>fo", file {producer = "vim.oldfile"}, "oldfiles"},
  {"<Leader>fs", file {args = args, try = {"git.file", "ripgrep.file"}}, "git-with-fallback"},
  {"<Leader>aaaa", file {combine = {"vim.buffer", "vim.oldfiles"}}}
}

Try and combine work and generate reasonable prompts without specification. You can specify a prompt suffix.

[camspiers]

But please remember this is just an API for generating functions that run snap.run so anyone could create their own API for doing the same, or create their own helpers in their vim config that do so. You can still do:

snap.maps {
  {"<Leader><Leader>", function ()
    -- my arbitrary function that runs snap.run
  end},
}

[camspiers]

Also worth mentioning that the third argument to snap.map or the third item in a table to snap.maps is optional.

So you can do:

snap.maps {
  {"<Leader><Leader>", file {producer = "ripgrep.file"}},
}

Or:

snap.map("<Leader><Leader>", file {producer = "ripgrep.file"})

Assuming you don't want to register your function as being available from the :Snap mycustomcommandname API.

[beauwilliams]

I think this new syntax is much nicer. Easier to understand at a glance. Your last example is definitely basic enough for anyone new to quickly figure out that you are mapping to ripgrep for file search

[camspiers]

Maybe defaults should be named, generate, config or, get

local config = require"snap.config"
local file = config.file:with {suffix = ">>"}
-- example with default config
snap.map("<Leader>f", file {producer = "ripgrep.file"})
-- example without config
snap.map("<Leader>b", config.file {producer = "vim.buffer"})

I'm not sure.

local generate = require"snap.generate"
local file = generate.file:with {suffix = ">>"}
-- example with default config
snap.map("<Leader>f", file {producer = "ripgrep.file"})
-- example without config
snap.map("<Leader>b", generate.file {producer = "vim.buffer"})

or:

local get = require"snap.get"
local file = get.file:with {suffix = ">>"}
-- example with default config
snap.map("<Leader>f", file {producer = "ripgrep.file"})
-- example without config
snap.map("<Leader>b", get.file {producer = "vim.buffer"})

Thoughts?

[beauwilliams]

I think config seems most reasonable to me

[beauwilliams]

I am not sure if this was doable before, but this is brilliant. This makes life so much easier having only one command, and the possibility to extend this is exciting.

{"<Leader>fs", file {args = args, try = {"git.file", "ripgrep.file"}}, "git-with-fallback"},

[beauwilliams]

I am not sure if you have implemented the combine yet, but it causes an error init.lua:70: attempt to call upvalue 'type' on 67b3a5c89ccc0a1c2dfd4418e854c01a358545c3

[camspiers]

Can you show me a little more how you are using combine?

[camspiers]

Oh I see the bug

[beauwilliams]

I noticed one other thing that I was not sure about, the 3rd argument (without going through the code), what is that for?

E.g I tried {"<Leader>f", vimgrep {}, "grep"}, and {"<Leader>f", vimgrep {}, "somethingrandom"},

And had the same result. Maybe I am missing something

[camspiers]

@beauwilliams should be fixed now.

The third argument to map, or the third item in the tables to snap.maps is optional,

if it is present then when you type :Snap and then hit tab, you will see the names that you provided in the third argument, and when select it an hit enter it will run that mapping.

See the screenshot in the first comment.

[beauwilliams]

@beauwilliams should be fixed now.

The third argument to map, or the third item in the tables to snap.maps is optional,

if it is present then when you type :Snap and then hit tab, you will see the names that you provided in the third argument, and when select it an hit enter it will run that mapping.

See the screenshot in the first comment.

Aha, yes I see now. That's great.

Can we make the mapping optional then? E.g, I have a command I only use rarely so don't want to set a mapping.

Something like this {"", vimgrep {args = args, prompt = "Grep"}, "grep"},

[camspiers]

For that just use the new register.command API.

[camspiers]

Alright I have renamed to config, apologies for any thrash when using the PR

[beauwilliams]

No worries at all. All working on my end after refactoring 👍

[beauwilliams]

@camspiers I noticed the layout.top could be improved slightly. Especially when reverse = false.

See here example from telescope

And what you currently see with snap

Can we move the prompt to the top for layout.top?

[camspiers]

I think I am going to change the way reverse works to put the input at the top because that is what people seem to want.

[beauwilliams]

I think I am going to change the way reverse works to put the input at the top because that is what people seem to want.

That makes more sense yeah. That's what I tried at first too you assume the results are kept near the prompt where you are typing

[akinsho]

@camspiers regarding the layout, and changing how reverse works, TBH I think reverse putting the input at the top makes sense since it truly is the reverse, although can the order of the selected results be changed in that case. ATM the item you have selected is at the top of the floating window whereas the input is underneath that window, so your eyes are looking in two different places to see what is actually currently selected. Telescope's default I think is nice here, which is that the selection is just above the input and at the bottom of the floating window.

EDIT: just realised that @beauwilliams's comment above says more or less the same thing about keeping the prompt and result near each other

[akinsho]

Also, the new changes are great, i.e. the snap.maps and being able to specify a try and combine key and greatly simplify the API.

One thing I noticed is that if you reload the Lua module where the mappings are specified rather than just reapplying the mapping, you get an error if you specified a command name, that the command has already been specified when next you try and use the mapping.

...m/site/pack/packer/opt/snap/lua/snap/common/register.lua:208: attempting to register duplicate command with name 'grep'

[bangedorrunt]

@camspiers thank you for your response. I just looked at your new proposed API and I love it. Your implementation is very close to my opinion so never mind, you are doing amazing good work!

Regarding to defaults, it's actually not too bad. But how about something more verbose such as get_root_config, get_default_config. It's up to you 😀

[akinsho]

A couple more things I noted (hopefully this is helpful, not spammy).

• If you place any items in the wrong part of the config (which is much simpler tbh) you get a generic file.producer is not valid error, which is still a handled error so already good, but for example in my case I had the prompt in the wrong table and was stumped for a while about why I was seeing this error. It's definitely an optimisation to handle right now but just noting it.

• The { "<leader>fo", file { combine = { "vim.buffer", "vim.oldfiles" } }, "recent-files" }, appears not to work, I get the file.producer invalid error, not sure if there's anything wrong but it's largely copy and pasted from the example in the PR description, I'm likely missing something.

• It might be a bit much since I know you can pass a directory to args but it might be a nice touch to add in a

  ---@type string|string[]
  cwd = my_directories

  similar to the telescope key, as well as via args (just as a nicety) and then combine that with the args under the hood?

[zetashift]

I also like the new proposed API, and a lot of the QoL it brings and that have been suggested!

So I don't really have anything worthy to add sadly ahha, thank you for this amazing plugin! :heart:

[camspiers]

Alright folks, the new snap.maps API has been merged into main. Please let me know if you have any problems, and if anyone has time, if you could look over the README to see what can be improved.

[gegoune]

Sorry for commenting here, does feel like it justifies the issue. I am not sure how to use initial_filter with latest API.

local vimgrep = snap.config.vimgrep:with {
  reverse = true,
  suffix = "",
  limit = 1000,
  layout = layout,
  preview = false,
}

snap.maps {
  { "<C-Space>", file { args = args, producer = "ripgrep.file", prompt = "Files" } },
  { "\\", vimgrep { args = args, prompt = "Grep" } },
  { "|", vimgrep { args = args, prompt = "Grep", initial_filter = vim.fn.expand "<cword>" } },
}

No word from under the cursor gets added to snap on |.

[camspiers]

It isn't supported yet, but I can add it, though the API will be different because it needs to be a function, I'm wondering give it is a common pattern if I also support a "current_word" boolean flag, as well as an arbitrary function for the filter.

[camspiers]

Implemented in https://github.com/camspiers/snap/commit/39d1af147e64ba5009727d4c1b538da255a72bef

You can now pass: filter_with = "cword", or if you have your mapping registered in visual mode then filter_with = "selection".

Otherwise you can just use filter which can be a string or a function that returns a string.

[camspiers]

You can also now override the mapping mode with modes

So:

snap.maps {
  {"<Leader>m", vimgrep {filter_with = "selection"}, {modes = "v"}}
}

This will enable you to bind a command to filter from the selection in visual mode.

[bangedorrunt]

@camspiers, Is it possible to use the following with new snap.config

snap.get'consumer.combine'(
  snap.get'producer.ripgrep.file'.args({}, "/directory1"),
  snap.get'producer.ripgrep.file'.args({}, "/directory2"),
)

Right now, I see combine only take a table of producers

[camspiers]

@babygau Producers can be an arbitrary function, so you should be able to do:

snap.maps {
  {"<Leader>whatever", file {
    combine = {
      snap.get'producer.ripgrep.file'.args({}, "/directory1"),
      snap.get'producer.ripgrep.file'.args({}, "/directory2")
    }
  }
}

If you can't I'm pretty sure it's a bug.

[bangedorrunt]

@camspiers, that worked, thank you for quick response. I'm pretty happy with new API so far

[camspiers]

Being completely honest, I didn't test combine 😅, but it uses the same approach as try and I tested try thoroughly.

Please let me know if it doesn't work. 🤩

[camspiers]

@babygau Awesome, thanks for updating me. 🤩

[akinsho]

@camspiers can the help producer be configured using the new API yet, or only via snap.run?

[camspiers]

Only snap.run I haven't hooked it up yet

[akinsho]

@camspiers regarding the duplicate command error issue I had a brief look at the code since I thought it was just an issue of the ! missing from command! but it seems it's how the command system is set up that stores a list of names to prevent duplicates. The issue I'm facing is that when I attempt to reload my config which I do automatically with an autocommand on saving the file where I set up packer, this causes the duplicate error.

I'm not sure how/what a good way to fix this, could the command just be replaced if there's a duplicate without erroring? this is similar to how command! would behave.

[leiserfg]

@akinsho https://github.com/camspiers/snap/pull/42/files#diff-f40c2c7846b2e9b765082c5a4a62ffca11351dfc94fe43f804460d53bc8eb747R63

That's the issue, maybe it should show a warning but not stop the registration.

[camspiers]

@babygau it should be require"snap.layout".bottom or snap.get"layout".bottom.

[akinsho]

I think a warning would be an improvement if it didn't stop the command from being registered since at present you can't reload snap without this, but I'm not sure if it's worth trying to prevent duplicates anyway. From vim's perspective, there can only be one matching command, so imo snap should just apply the commands it gets.

[camspiers]

@akinsho I can change the behavior. Fixed https://github.com/camspiers/snap/commit/4e0bd091a809d197f2265e96bc3714c39f6403be

[camspiers]

@akinsho It's there because usually it is a mistake in someones config. They aren't duplicate commands so much as duplicate names of commands attempting to be made available to :Snap ...

[bangedorrunt]

@babygau it should be require"snap.layout".bottom or "snap.get"layout".bottom.

Thanks, that did it!

Could you check if reverse layout has no effect with snap.config.vimgrep? If you look at the screenshot, the prompt is still at the bottom.

[ahmedelgabri]

@camspiers Sorry I was very busy during the week, so didn't have time to check this.

I went through the thread & I don't think I have much more to add to others. I like the API too, but I have one question. How can I pass custom mapping using the new API?

snap.run({mappings = {["view-toggle-hide"] = {"<C-a>"}}})