axkirillov / hbac.nvim

Heuristic buffer auto-close
MIT License
197 stars 10 forks source link

pass telescope picker opts properly #9

Closed al-ce closed 11 months ago

al-ce commented 1 year ago

As @ditsuke and @technicalpickles have mentioned, the Telescope picker isn't implementing opts that a user might expect from a buffer picker, which is more or less the picker we were trying to imitate. This is my fault for not looking back at the buffer picker as a model towards the end of the PR.

Here's the structure of the builtin buffer picker (link)

  pickers
    .new(opts, {
      prompt_title = "Buffers",
      finder = finders.new_table {
        results = buffers,
        entry_maker = opts.entry_maker or make_entry.gen_from_buffer(opts),
      },
      previewer = conf.grep_previewer(opts),
      sorter = conf.generic_sorter(opts),
      default_selection_index = default_selection_idx,
    })
    :find()

and here's the hbac 'pin' picker as of release v1.2.0 (link)

M.pin_picker = function(opts)
  local attach_mappings = require("hbac.telescope.attach_mappings")
  local make_finder = require("hbac.telescope.make_finder").make_finder
  opts = opts or {}
    pickers
      .new(opts, {
        prompt_title = "Hbac Pin States",
        finder = make_finder(),
        sorter = telescope_conf.generic_sorter(opts),
        attach_mappings = attach_mappings.attach_mappings,
        previewer = telescope_conf.file_previewer(opts),
      })
      :find()
end

My oversight was not giving make_finder() the opts table and checking for the relevant builtin buffer picker opts in the function that builds the finder results (get_entries()). Since the sorter and the previewer functions are called straight from Telescope, I just stopped thinking about the opts.

So, we need to pass the opts table to the function that builds the finder. The function that checks the opts and filters the buffer list is unexposed in the Telescope code unfortunately. The blunt thing to do would be to just copy that code into the make_finder function and make any relevant changes (this is what I'm trying on my local repo). (EDIT: since we're refreshing the picker after most actions, this make_finder call needs the opts too)

The builtin buffer picker also has the default_selection_index key which is needed for setting the row position correctly if sort_lastused is true and ignore_current_buffer is false, so we would need to access that after its value is updated.

Sorry for the oversight, I hope the indication to the code is useful.

al-ce commented 1 year ago

Wanted to follow up since I think this might be related to #12. I didn't mean to leave this issue hanging. @ditsuke mentioned in this comment that he might have opened a PR to implement the sort-by-mru, but he was also busy with exams at the time.

I added this a while back in my experimental fork with these commits https://github.com/al-ce/hbac.nvim/commit/5b9a4e27d073cf66b6e72ed19dd413649c57e48d https://github.com/al-ce/hbac.nvim/commit/94e4a1656694123a7c5b67d0827fabc6d452bb68 in different branches, but I didn't open a PR because it uses the not-so-elegant fix I suggested above, to basically copy over the Telescope-opts code and modify it for the Hbac picker. This is what I have in my setup config for my keymap bind:

function()
  local default_opts = {
    show_all_buffers = true,
    sort_mru = true,
    sort_lastused = false,
    ignore_current_buffer = false,
    cwd_only = false,
 }

  require("hbac").telescope(default_opts)
end

@technicalpickles @axkirillov do you have any thoughts on this? Not sure if this would address the request in #12

axkirillov commented 1 year ago

@al-ce ah, I totally forgot about this. I think it's totally fine to copy telescope code over here.

ditsuke commented 1 year ago

@al-ce thank you for opening the PR! Apologies for my inaction, I really wanted to see this through, but exams followed by a detox break and then ensuing academic and work obligations have had me spending a very limited time in the nvim ecosystem lately, and this issue totally slipped past my memory. Looking forward to using the improved picker

al-ce commented 1 year ago

@ditsuke No worries, I figured you were busy! I'm glad you got a break for a bit and I hope work and school are going well. I've been learning about lots of interesting repos from what you've been starring over the past few months as they show up in my feed, so thanks for that! Hope to work with you again soon!

axkirillov commented 11 months ago

@al-ce Can this be closed?