camspiers / snap

A fast finder system for neovim.
The Unlicense
484 stars 16 forks source link

[Minor] Reverse layout? #6

Closed ahmedelgabri closed 3 years ago

ahmedelgabri commented 3 years ago

It will be great if we can have a layout that has the input on the top, my brain prefers this more.

camspiers commented 3 years ago

I think this makes sense, I'll take a look at how I can do it performatly

camspiers commented 3 years ago

Added initial support in this commit:

https://github.com/camspiers/snap/commit/8628478288bb64ed59989910e736879054facda3

If folks could test and see how it goes that would be awesome. You can activate reversed layout with:

snap.run { reverse = true, ... your other normal options ...}
camspiers commented 3 years ago

Btw, I realize that I didn't do exactly what you requested, instead I reversed the results view such that the highest matching items are at the bottom and the view begins with the last item in the buffer selected. I think this makes things more intuitive.

ahmedelgabri commented 3 years ago

Btw, I realize that I didn't do exactly what you requested, instead I reversed the results view such that the highest matching items are at the bottom and the view begins with the last item in the buffer selected. I think this makes things more intuitive.

Thanks for looking into this, but that's actually more confusing to me. (I'm sorry I don't mean to come across as negative) my main problem is the prompt placement not the order of the results.

camspiers commented 3 years ago

It's not negative! It's great user feedback. I'll reopen the issue 😊

akinsho commented 3 years ago

FWIW, I actually really like the new reverse mode. I'd just been searching through issues to see if it had come up, found this and tried it out. Regarding testing it and feeding back, it's a little odd not sure it's actually a problem just strange UI but after typing in a query and the results are narrowed down, The results don't seem to be anchored down to the bottom image

The space is always beneath the results, not above it.

camspiers commented 3 years ago

Hmm, interesting, thanks for the report, I am not seeing that behavior on my setup, so I need to do some more testing

akinsho commented 3 years ago

The exact function I'm using to reproduce this is

        snap.register.map({ "n" }, { "<leader>ff" }, function()
          snap.run {
            reverse = true,
            producer = fzf(
              snap.get "consumer.try"(
                snap.get "producer.git.file",
                snap.get("producer.ripgrep.file").hidden
              )
            ),
            select = snap.get("select.file").select,
            multiselect = snap.get("select.file").multiselect,
            views = { snap.get "preview.file" },
          }
        end)

In case that helps.

bangedorrunt commented 3 years ago

@camspiers one more thing, is it possible to replicate fzf.vim bottom layout?. I'm not a big fan of floating window though

g:fzf_layout = { 'down': '~40%' }
camspiers commented 3 years ago

@babygau Floating isn't optional, but you can provide your own custom layout function to achieve the same kind of thing:

snap.run {
  layout = function()
      local lines = vim.api.nvim_get_option("lines")
      local height = math.floor((lines * 0.5))
      local width = (vim.api.nvim_get_option("columns") - 4)
      return {col = 0, row = (lines - height - 4), height = height, width = width}
  end,
  ... other options
}
bangedorrunt commented 3 years ago

Legend, thank you for your suggestion 💯

gegoune commented 3 years ago

Doesn't feel justified to open another issue but I wanted to ask how could I configure the border for each panel? Is it possible at the moment?

Thanks, that is blazingly fast picker! I though fzf was fast.

Edit: I can see that border is not being passed to function opening window: https://github.com/camspiers/snap/blob/9bd645c1a1c11e816de83bd64a46a7413be808b2/fnl/snap/common/window.fnl#L3 - is this something you would accept PR for?

camspiers commented 3 years ago

@gegoune I would definitely accept a PR. I am yet to do a consolidation pass over the views, there is duplication there but I have been waiting to find where the common patterns are so the abstraction is done well.

The files where a border will need to be passed to window.create are:

view/input.fnl view/results.fnl view/view.fnl

The approach in all 3 are the same, there is a layout function which produces a layout-config that is passed to window.create. Each of these will need to pass through the border such that when you return custom borders from your layout function (passed to snap.run) they will be passed down to each window.create call.

Let me know if you need guidance. And if you see a common pattern in there and want to abstract feel free to.

camspiers commented 3 years ago

I need to add this to the docs but I have just introduced a new api:

snap.create(config, defaults)

It's really simple, when called it returns a function that calls snap.run with the two configs passed in merged.

This means that you can simplify creation of mappings:

The following old pattern:

local snap = require'snap'
snap.register.map({"n"}, {"<Leader>f"}, function ()
  snap.run {
    producer = snap.get'consumer.fzy'(snap.get'producer.ripgrep.file'),
    select = snap.get'select.file'.select,
    multiselect = snap.get'select.file'.multiselect
  }
end)

Can now become:

-- import snap
local snap = require'snap'

-- custom layout function
local function layout()
  -- my custom layout config
end

-- this config will be merged in each
local defaults = {layout = layout, reverse = true}

-- register a mapping
snap.register.map({"n"}, {"<Leader>f"}, snap.create(function ()
  return {
    producer = snap.get'consumer.fzf'(snap.get'producer.ripgrep.file'),
    select = snap.get'select.file'.select,
    multiselect = snap.get'select.file'.multiselect
  }
end, defaults))

If you want to pass the same defaults in all your mappings then you can do something like this:

-- import snap
local snap = require'snap'

-- Here you can define some defaults, like your default layout function
-- and the reverse prop if you like it
local function layout()
  -- my custom layout config
end

-- this config will be merged in each
local function create(config)
  return snap.create(config, {layout = layout, reverse = true})
end

-- register a mapping
snap.register.map({"n"}, {"<Leader>f"}, create(function
  return {
    producer = snap.get'consumer.fzf'(snap.get'producer.ripgrep.file'),
    select = snap.get'select.file'.select,
    multiselect = snap.get'select.file'.multiselect
  }
end))
camspiers commented 3 years ago

From this (and all the other APIs) it might be apparent that I am avoiding adding state to snap, in snap you don't make some global configuration call, like snap.configure-snap-this-way, or ripgrep.file.configure-all-ripgreps-this-way. Instead the philosophy is to avoid global state, and instead make convenient APIs over configuration, so everything still boils down to a stateless call to snap.run with a stateless config.

This might be unintuitive for users coming from other vim plugins, or from telescope for example, but I'm a strong advocate against stateful configuration APIs where you change global state, as I believe this is a source of a lot of bugs, leads to weird API decisions with override flags, giant configuration objects with many special keys that do a grabbag of unrelated configuration, and can cause plugins (e.g. snap plugins) to need to understand global configuration and user configuration in order to interact properly with each other.

But I do need to invest some time making the APIs for this approach convenient. Please feel free to communicate what feels like too much boilerplate, as that is where I will focus.

camspiers commented 3 years ago

Crap, I didn't quite think through the create API and might need to change it. Will update soon. Apologies.

camspiers commented 3 years ago

I've changed the API, but I am not really happy with it. The API I introduced didn't really work because it meant the config was static across invocations, and we don't really want that, it means for example cache consumers will keep their memory across snap runs, and we don't want that with respect to memory not freeing, additionally producers will producer different results when running in different directories (cwds). My fault for not thinking it through, apologies.

bangedorrunt commented 3 years ago

Does it mean snap.create() would be deprecated soon?

camspiers commented 3 years ago

@babygau I will keep it, but it isn't what I was hoping to create because you still need to define a function to pass to it which I don't really like. I still think it adds some value in that it more easily allows mixing in a default config to your mappings.

camspiers commented 3 years ago

If you have any suggestions I am very open to it. I am looking at making available some defaults, they will be functions that run common patterns, but I am contemplating how best to make them configurable.

The way that I am thinking about it is that the there is a default for each type of thing you want to select, e.g. one for file, one for vimgrep.

snap.default.file would look something like this, in fennel:

(fn create [config]
  (local producer
    (match (or config.producer "ripgrep")
      :ripgrep (snap.get :producer.ripgrep.file)
      (where a (= (type a) :function)) a))
  (local consumer
    (match (or config.consumer "fzf")
       :fzf (snap.get :consumer.fzf)
       :fzy (snap.get :consumer.fzy)))
    {:prompt (or config.prompt :Files>)
     :producer (consumer producer)
     :select (. (snap.get :select.file) :select)
     :multiselect (. (snap.get :select.file) :multiselect)
     :views [(snap.get :preview.file)]})

(fn snap.defaults.file [config]
  (fn [] (snap.run (create config))))

So something like, in lua:

snap.map(
  "n",
   "<Leader>f",
   snap.defaults.file {}
)

or to use fzy:

snap.map(
  "n",
   "<Leader>f",
   snap.defaults.file {
     prompt = "FZY>",
     consumer = "fzy"
   }
)

or to use a custom producer with fzf:

snap.map(
  "n",
   "<Leader>f",
   snap.defaults.file {
     producer = myproducer
   }
)
camspiers commented 3 years ago

Then there would be a snap.defaults.vimgrep:

(fn create [config]
  (local producer
    (match (or config.producer "ripgrep")
      :ripgrep (snap.get :producer.ripgrep.vimgrep)
      (where a (= (type a) :function)) a))
    {:prompt (or config.prompt :Grep>)
     :producer (if config.limit ((snap.get :consumer.limit) config.limit producer) producer)
     :select (. (snap.get :select.vimgrep) :select)
     :multiselect (. (snap.get :select.vimgrep) :multiselect)
     :views [(snap.get :preview.vimgrep)]})

(fn snap.defaults.vimgrep [config]
  (fn [] (snap.run (create config))))
camspiers commented 3 years ago

Where in lua you would run:

snap.map(
  "n",
   "<Leader>f",
   snap.defaults.vimgrep {
     limit = 10000
   }
)
camspiers commented 3 years ago

Each default would have its config documented, and different options would be available for the different appropriate things you would commonly configure.

camspiers commented 3 years ago

See the PR https://github.com/camspiers/snap/pull/42 for the solution I have arrived at.

bangedorrunt commented 3 years ago

@camspiers, reverse layout sometimes produced this error while I'm scrolling through result list. It looks like the file name is not showned correctly. Ex: .vi instead of .vim or .lu instead of .lua

Screen Shot 2021-06-22 at 10 34 37 pm
gegoune commented 3 years ago

Let me know if you need guidance. And if you see a common pattern in there and want to abstract feel free to.

I have tried but failed due to having zero fennel knowledge. Didn't manage to make border parameter passable with default value. :(

camspiers commented 3 years ago

Input is now shifted to top, this might cause thrash because I am changing the behaviour of it, but it seems to be what people expect or want.

yujinyuz commented 3 years ago

@camspiers I was quite wondering why my snap has changed but can we also have an option wherein the cursor is at the bottom (similar to what the behavior was before when using reverse = true)

Kinda like I want the current layout like this but it should start at the bottom

Screen Shot 2021-06-27 at 08 01 01 Screen Shot 2021-06-27 at 07 59 40
bangedorrunt commented 3 years ago

Agree with @yujinyuz, this would be great if we could have a layout similar to FZF.vim with prompt and selected items in the bottom and fuzzy search processed from bottom-up.

camspiers commented 3 years ago

I need to figure out how to do it in a more performant way. My initial implementation wasn't very good, so I changed the behavior of reverse to put the input at the top like many people request. Sorry for the thrash.