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

feat(telescope): add lvim.builtin.telescope.theme #3548

Merged

Conversation

LostNeophyte
Copy link
Member

Description

lvim.builtin.telescope.pickers will now be autocompleted

partially fixes #3406

How Has This Been Tested?

image

Copy link
Collaborator

@kylo252 kylo252 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with splitting the theme refactor to another PR if you don't feel like dealing with it tho :)

lua/lvim/core/telescope.lua Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
@LostNeophyte
Copy link
Member Author

the global theme should be more easily configurable, here's an example of how to do that: https://github.com/kylo252/lvim/blob/d381278cf22fdf6f7a7cc59da10aeced315f082f/lua/user/telescope.lua#L60-L31

theme = "ivy" here doesn't have any effect though, right? a search for telescope.defaults.theme in telescope.txt doesn't return anything.

the suggested changes would change the theme to horizontal in the affected pickers and enable the preview in some, is that change intentional?

I'm okay with splitting the theme refactor to another PR if you don't feel like dealing with it tho :)

I'd be happy to do it, not sure what to do though

@kylo252
Copy link
Collaborator

kylo252 commented Dec 1, 2022

theme = "ivy" here doesn't have any effect though, right? a search for telescope.defaults.theme in telescope.txt doesn't return anything.

correct, it's more of a way for me to keep track of which theme is being used 😅

that code block is basically my way of inlining this functions: https://github.com/nvim-telescope/telescope.nvim/blob/cea9c75c19d172d2c6f089f21656019734a615cf/lua/telescope/themes.lua#L98-L135

he suggested changes would change the theme to horizontal in the affected pickers and enable the preview in some, is that change intentional?

The theme should be controlled globally to make it more consistent. As for the preview, we should definitely drop them from this table since we don't want those changes to be global.

We can preserve the current behavior, where desired, in the mapping itself

        f = { "<cmd>Telescope find_files previewer=false theme=dropdown<cr>", "Find File" },

I'd be happy to do it, not sure what to do though

let's figure it out together :)

related #2286


cc: @abzcoding

@LostNeophyte
Copy link
Member Author

I've added lvim.builtin.telescope.defaults.theme and moved previewer=false to whichkey, what do you think?

@LostNeophyte
Copy link
Member Author

and how about doing Various themes for the main UIs as a separate plugin that you require in your config and it applies some settings?

Copy link
Collaborator

@kylo252 kylo252 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and how about doing Various themes for the main UIs as a separate plugin that you require in your config and it applies some settings?

introducing a new theme should be done as a plugin, which is the idea in #2286 (comment) since it requires extra fiddling with highlight groups, see https://github.com/abzcoding/lvim/blob/575310300e72a5ff14c424e21aabe7e43baefa9e/lua/user/theme.lua#L437-L497, but I don't think we need a plugin just for selecting the theme.

lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
lua/lvim/core/telescope.lua Outdated Show resolved Hide resolved
Copy link
Collaborator

@kylo252 kylo252 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@kylo252 kylo252 added enhancement New feature or request enhancement:docs Documentation improvements labels Dec 7, 2022
@kylo252 kylo252 changed the title feat(telescope): better lvim.builtin.telescope completion feat(telescope): add lvim.builtin.telescope.theme Dec 7, 2022
Copy link
Member

@abzcoding abzcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@kylo252 kylo252 merged commit 245a71c into LunarVim:master Dec 8, 2022
tomazursic pushed a commit to tomazursic/LunarVim that referenced this pull request Dec 9, 2022
* upstream/master:
  feat(telescope): add `lvim.builtin.telescope.theme` (LunarVim#3548)
@LostNeophyte LostNeophyte deleted the feat/telescope-builtin-completion branch December 13, 2022 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement:docs Documentation improvements enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Telescope make pickers more easily configurable
3 participants