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

Adding a previewer option to colorscheme picker #2966

Closed
shreyas-a-s opened this issue Mar 2, 2024 · 19 comments · Fixed by #3097
Closed

Adding a previewer option to colorscheme picker #2966

shreyas-a-s opened this issue Mar 2, 2024 · 19 comments · Fixed by #3097
Labels
enhancement Enhancement to performance, inner workings or existent features good first issue Good for newcomers

Comments

@shreyas-a-s
Copy link

shreyas-a-s commented Mar 2, 2024

Is your feature request related to a problem? Please describe.
Currently the option to customise colorscheme is the 'enable_preview' parameter and it controls whether selecting a colorscheme from list changes colorscheme. And changing colorscheme causes both the previewer as well as the currently open buffers to change colorscheme. I would like the addition of a 'proper' previewer option so that I can set enable_preview=true and previewer=false and selecting a colorscheme changes the colorscheme of currenly open buffers while the previewer is kept disabled. Currently when I run :Telescope colorscheme previewer=false enable_preview=true, somehow both options are coupled together so that the enable_preview functionality (changing colorscheme of currently open buffers) is connected to previewer functionality.

Describe the solution you'd like
'Decouple' previewer and enable_preview options to colorscheme picker so that they can be changed independently.

Describe alternatives you've considered
No alternatives as far as I thought.

Additional context
Nothing.

I know it's a bit confusing and I can provide more context if you'd like. Thanks for reading through this message.

@shreyas-a-s shreyas-a-s added the enhancement Enhancement to performance, inner workings or existent features label Mar 2, 2024
@jamestrew
Copy link
Contributor

jamestrew commented Mar 2, 2024

Yeah I agree. Shouldn't be too hard.
I'd be willing to give guidance if anybody is interested. Otherwise, I'll try to get to this when I have some time.

@jamestrew jamestrew added the good first issue Good for newcomers label Mar 2, 2024
@shreyas-a-s
Copy link
Author

I tried to make some changes with my limited knowledge of lua that I got from tweaking neovim. But it is found to be not enough for the job. It would be nice if someone with more expertise could take a look and make further improvements when convenient.

@shreyas-a-s
Copy link
Author

shreyas-a-s commented Mar 4, 2024 via email

@Jayanth-Parthsarathy
Copy link

Hey @jamestrew, I'm eager to contribute, but I'm still getting the hang of Lua myself. I've been looking through the code for telescope colorschemes. Here's what I've understood so far:

local picker = pickers.new(opts, {
  prompt_title = "Change Colorscheme",
  finder = finders.new_table {
    results = colors,
  },
  sorter = conf.generic_sorter(opts),
  previewer = previewer,
  attach_mappings = function(prompt_bufnr)
    actions.select_default:replace(function()
      local selection = action_state.get_selected_entry()
      if selection == nil then
        utils.__warn_no_selection "builtin.colorscheme"
        return
      end

      actions.close(prompt_bufnr)
      need_restore = false
      vim.cmd.colorscheme(selection.value)
    end)
    return true
  end,
}, highlight_colors)

I think passing the highlight colors of the old theme might be beneficial, but I'm not entirely sure how to proceed from here. I've parsed the highlight_colors and turned them into a table with the highlight group and the corresponding color hex value. Then, I passed it as an optional argument to the new_picker and the default_create_layout function.

If you could provide some guidance, I believe I can resolve this issue.

@jamestrew
Copy link
Contributor

jamestrew commented Mar 4, 2024

My approach would be to take advantage of the action to cycle through entries and change the colorscheme when a new entry is "selected".
This relates to the shift_selection action in telescope.actions.set (actions.move_selection_next and related actions all call this).
These actions are all fancy lua metatables that Telescope has added methods to (replace, enhance, etc - defined in telescope.actions.mt).
So similar to how we're replace-ing the select_default behavior, we can enhance the shift_selection's post selection behavior.

Something like so

      attach_mappings = function()
        actions.select_default:replace(...) -- no change
        action_set.shift_selection:enhance({ post = function()
          -- set colorscheme to the currently selected entry one
          -- this is for when someone cycles through the result
        end })
        actions.close:enhance({ post = function()
          -- reset back to original, pre-telescope colorscheme after closing telescope
        end })
        return true
      end,
      on_complete = {
        function()
          -- also set colorscheme to the currently selected entry
          -- this is for when someone prompts and searches for a result
        end,
      },

@Jayanth-Parthsarathy
Copy link

Hey @jamestrew. I assume you mean something like this right?

  local picker = pickers.new(opts, {
    prompt_title = "Change Colorscheme",
    finder = finders.new_table {
      results = colors,
    },
    sorter = conf.generic_sorter(opts),
    previewer = previewer,
    attach_mappings = function(prompt_bufnr)
      actions.select_default:replace(function()
        local selection = action_state.get_selected_entry()
        if selection == nil then
          utils.__warn_no_selection "builtin.colorscheme"
          return
        end

        actions.close(prompt_bufnr)
        need_restore = false
        vim.cmd.colorscheme(selection.value)
      end)
      action_set.shift_selection:enhance {
        post = function()
          local selection = action_state.get_selected_entry()
          if selection == nil then
            utils.__warn_no_selection "builtin.colorscheme"
            return
          end
          need_restore = true
          vim.cmd.colorscheme(selection.value)
        end,
      }
      actions.close:enhance {
        post = function()
          if need_restore then
            vim.cmd.colorscheme(before_color)
          end
        end,
      }
      return true
    end,
    on_complete = {
      function()
        local selection = action_state.get_selected_entry()
        if selection == nil then
          utils.__warn_no_selection "builtin.colorscheme"
          return
        end
        need_restore = true
        vim.cmd.colorscheme(selection.value)
      end,
    },
  })

@jamestrew
Copy link
Contributor

That looks good at first glance.

@MovieMaker93
Copy link
Contributor

Is still open? Can I take it?

@MovieMaker93
Copy link
Contributor

Hey @jamestrew. I assume you mean something like this right?

  local picker = pickers.new(opts, {
    prompt_title = "Change Colorscheme",
    finder = finders.new_table {
      results = colors,
    },
    sorter = conf.generic_sorter(opts),
    previewer = previewer,
    attach_mappings = function(prompt_bufnr)
      actions.select_default:replace(function()
        local selection = action_state.get_selected_entry()
        if selection == nil then
          utils.__warn_no_selection "builtin.colorscheme"
          return
        end

        actions.close(prompt_bufnr)
        need_restore = false
        vim.cmd.colorscheme(selection.value)
      end)
      action_set.shift_selection:enhance {
        post = function()
          local selection = action_state.get_selected_entry()
          if selection == nil then
            utils.__warn_no_selection "builtin.colorscheme"
            return
          end
          need_restore = true
          vim.cmd.colorscheme(selection.value)
        end,
      }
      actions.close:enhance {
        post = function()
          if need_restore then
            vim.cmd.colorscheme(before_color)
          end
        end,
      }
      return true
    end,
    on_complete = {
      function()
        local selection = action_state.get_selected_entry()
        if selection == nil then
          utils.__warn_no_selection "builtin.colorscheme"
          return
        end
        need_restore = true
        vim.cmd.colorscheme(selection.value)
      end,
    },
  })

Have you tried this code? What is missing?

@MovieMaker93
Copy link
Contributor

Is your feature request related to a problem? Please describe. Currently the option to customise colorscheme is the 'enable_preview' parameter and it controls whether selecting a colorscheme from list changes colorscheme. And changing colorscheme causes both the previewer as well as the currently open buffers to change colorscheme. I would like the addition of a 'proper' previewer option so that I can set enable_preview=true and previewer=false and selecting a colorscheme changes the colorscheme of currenly open buffers while the previewer is kept disabled. Currently when I run :Telescope colorscheme previewer=false enable_preview=true, somehow both options are coupled together so that the enable_preview functionality (changing colorscheme of currently open buffers) is connected to previewer functionality.

Describe the solution you'd like 'Decouple' previewer and enable_preview options to colorscheme picker so that they can be changed independently.

Describe alternatives you've considered No alternatives as far as I thought.

Additional context Nothing.

I know it's a bit confusing and I can provide more context if you'd like. Thanks for reading through this message.

If you don't mind could you give more context about the issue? Some screenshot of the desired output would be nice

@shreyas-a-s
Copy link
Author

I don't mind at all. I'll provide more info.

@shreyas-a-s
Copy link
Author

So at first I'll try to explain it in words and if that becomes not enough, I could provide the screenshots or even a video as well.

Now coming to the use case:

Me being a minimalist, I really like to reduce the things on neovim window to get a clean experience. As part of that, I disabled previewer functionality of Telescope by using this line in my telescope.lua file. You could consider it be the same as running :Telescope picker_name previewer=false.

For all the other pickers this change didn't affect in any way. All their functionalities worked the same as before. But for the colorscheme picker this affected the live-preview functionality. What I mean by that is, it now doesn't cycle though the different colorschemes when moving through the Telescope list of colorschemes. I tried to explicitely enable the functionality using the line enable_preview = true for the picker but nothing changed.

So what I concluded is that, the enable_preview option of colorscheme picker is liked to the preview/previewer option. Unlinking them is what I am proposing.

Now you may ask, how to check if they are linked or unlinked?

Good question.

Run this command:

Telescope colorscheme previewer=false enable_preview=true

If colors in neovim window changes when moving through the list of colorschemes, the feature is working and the unlinking is complete. Otherwise they are still linked.

@shreyas-a-s
Copy link
Author

Hey @jamestrew. I assume you mean something like this right?

  local picker = pickers.new(opts, {
    prompt_title = "Change Colorscheme",
    finder = finders.new_table {
      results = colors,
    },
    sorter = conf.generic_sorter(opts),
    previewer = previewer,
    attach_mappings = function(prompt_bufnr)
      actions.select_default:replace(function()
        local selection = action_state.get_selected_entry()
        if selection == nil then
          utils.__warn_no_selection "builtin.colorscheme"
          return
        end

        actions.close(prompt_bufnr)
        need_restore = false
        vim.cmd.colorscheme(selection.value)
      end)
      action_set.shift_selection:enhance {
        post = function()
          local selection = action_state.get_selected_entry()
          if selection == nil then
            utils.__warn_no_selection "builtin.colorscheme"
            return
          end
          need_restore = true
          vim.cmd.colorscheme(selection.value)
        end,
      }
      actions.close:enhance {
        post = function()
          if need_restore then
            vim.cmd.colorscheme(before_color)
          end
        end,
      }
      return true
    end,
    on_complete = {
      function()
        local selection = action_state.get_selected_entry()
        if selection == nil then
          utils.__warn_no_selection "builtin.colorscheme"
          return
        end
        need_restore = true
        vim.cmd.colorscheme(selection.value)
      end,
    },
  })

Have you tried this code? What is missing?

Ohhh. I haven't really checked out this code before, but now that I did test it I think it works. This is the video after inserting the code into telescope.nvim/lua/telescope/builtin/__internal.lua:

2024-05-06_23-55-38.mp4

As you can see in the video, when previewer is set to false, the live-preview functionality is not affected.

If you want to see what was the issue, see this. This is the video before inserting that code:

2024-05-07_00-02-48.mp4

As you can see, when previewer was set to false, the live-preview functionality was affected.

So I think it's fixed then?

@MovieMaker93
Copy link
Contributor

Ok, nice; if it is fixed, @Jayanth-Parthsarathy, would you mind opening a PR?

@Jayanth-Parthsarathy
Copy link

Jayanth-Parthsarathy commented May 7, 2024

Hey @MovieMaker93 . I have not made any progress on this. Please feel free to work on this.

@MovieMaker93
Copy link
Contributor

MovieMaker93 commented May 7, 2024

Hey @MovieMaker93 . I have not made any progress on this. Please feel free to work on this.

I have tried your code, and seems to work as expected. Do you have any suggestion or doubts about this implementation @jamestrew ?

@jamestrew
Copy link
Contributor

@MovieMaker93 I don't have any concerns off the top of my head. Are you able to open a PR? I'd be better able to see the change and offer feedback with a PR.

Otherwise, I can also try to implement this myself over the next few days.

@MovieMaker93
Copy link
Contributor

@MovieMaker93 I don't have any concerns off the top of my head. Are you able to open a PR? I'd be better able to see the change and offer feedback with a PR.

Otherwise, I can also try to implement this myself over the next few days.

Yes i can do it, maybe you have to assign the issue (actually i have denied error when pushing new branch).

@Jayanth-Parthsarathy
Copy link

@MovieMaker93 I don't have any concerns off the top of my head. Are you able to open a PR? I'd be better able to see the change and offer feedback with a PR.

Otherwise, I can also try to implement this myself over the next few days.

Yes i can do it, maybe you have to assign the issue (actually i have denied error when pushing new branch).

If I am not mistaken this might be because you need to fork the repo before submitting a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to performance, inner workings or existent features good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants