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

Refactor builtin into smaller modules #287

Merged
merged 2 commits into from
Nov 26, 2020

Conversation

Conni2461
Copy link
Member

@Conni2461 Conni2461 commented Nov 24, 2020

Should be good but someone should take a look at this so i don't wake up with a lot of angry messages. New structure:

builtin.live_grep = require('telescope.builtin.files').live_grep
builtin.grep_string = require('telescope.builtin.files').grep_string
builtin.find_files = require('telescope.builtin.files').find_files
builtin.fd = builtin.find_files
builtin.treesitter = require('telescope.builtin.files').treesitter
builtin.current_buffer_fuzzy_find = require('telescope.builtin.files').current_buffer_fuzzy_find
builtin.tags = require('telescope.builtin.files').tags
builtin.current_buffer_tags = require('telescope.builtin.files').current_buffer_tags

builtin.git_files = require('telescope.builtin.git').files
builtin.git_commits = require('telescope.builtin.git').commits
builtin.git_bcommits = require('telescope.builtin.git').bcommits
builtin.git_branches = require('telescope.builtin.git').branches
builtin.git_status = require('telescope.builtin.git').status

builtin.builtin = require('telescope.builtin.internal').builtin
builtin.planets = require('telescope.builtin.internal').planets
builtin.commands = require('telescope.builtin.internal').commands
builtin.quickfix = require('telescope.builtin.internal').quickfix
builtin.loclist = require('telescope.builtin.internal').loclist
builtin.oldfiles = require('telescope.builtin.internal').oldfiles
builtin.command_history = require('telescope.builtin.internal').command_history
builtin.vim_options = require('telescope.builtin.internal').vim_options
builtin.help_tags = require('telescope.builtin.internal').help_tags
builtin.man_pages = require('telescope.builtin.internal').man_pages
builtin.reloader = require('telescope.builtin.internal').reloader
builtin.buffers = require('telescope.builtin.internal').buffers
builtin.colorscheme = require('telescope.builtin.internal').colorscheme
builtin.marks = require('telescope.builtin.internal').marks
builtin.registers = require('telescope.builtin.internal').registers
builtin.keymaps = require('telescope.builtin.internal').keymaps
builtin.filetypes = require('telescope.builtin.internal').filetypes
builtin.highlights = require('telescope.builtin.internal').highlights

builtin.lsp_references = require('telescope.builtin.lsp').references
builtin.lsp_document_symbols = require('telescope.builtin.lsp').document_symbols
builtin.lsp_code_actions = require('telescope.builtin.lsp').code_actions
builtin.lsp_workspace_symbols = require('telescope.builtin.lsp').workspace_symbols

@Conni2461
Copy link
Member Author

Conni2461 commented Nov 24, 2020

Oh btw @tjdevries I made sure that the sorter is always loaded with conf. So this would partially fix #277. (Everything besides registers because we only grep for one char there.)

We also don't have to remember to do opts = opts or {} at the beginning of every function. All modules besides builtin/init.lua have a apply_checks(mod) which does that.

Also also do we wanna check in lsp.lua if a lsp is attached before actually running any of these functions?

@Conni2461
Copy link
Member Author

@tjdevries Need your help for a sec with one change. Because we do apply_checks(mod) for all builtin modules now and overwrite all functions mod[k], the previewer of builtin.builtin will no longer work correctly. The line number will always be where mod[k] = function... happens. Do you know what i mean?
I thought maybe some metatable magic could solve this but i haven't figured it out yet. Also we could just get the line number with vim.fn.search or treesitter (but i don't like requiring treesitter for that)

@tjdevries
Copy link
Member

Ah, I hadn't thought of that part of it.

One thing we could do is we could just save that as some metadata with the functions. But I think that's a bit hacky and perhaps error prone.

vim.fn.search does not seem like a bad plan, I will have to think of it a bit more though. Perhaps a layer of indirection between the two could make it work, but I'm not 100% sure yet. We can always do something different for the previewer as well.

For example, something cool would be we could write some documentation for the functions or something and then show the corresponding help.... I should really work on my doc generator :)

@Conni2461
Copy link
Member Author

Conni2461 commented Nov 25, 2020

I hadn't thought of that either. I would like to merge this today, so that we can keep the number of people who need to rebase as small as possible.
So either we merge without a fix, i will create an issue and someone fixes it later, or i will write a new preview using vim.fn.search(). Up to you.

Oh and yeah i have seen some of that doc generator based on treesitter, looks very promising and really helpful. So i am exited about that one.

@Conni2461
Copy link
Member Author

Conni2461 commented Nov 25, 2020

@tjdevries Fixed the builtin.builtin previewer. Uses vim buffer and vim.fn.search(). We might have to clean up previewers and enable vim buffers for more pickers (as opt in). Might do that next.

@tami5 Could you test it for a second. Thanks.

@tjdevries
Copy link
Member

If it works, then you can merge. Don't have time to test today really. We can always poke around the previewer later. I agree should not keep this branch open for long.

@kkharji
Copy link
Member

kkharji commented Nov 26, 2020

Oh wow, it works really well. You've also fixed a lot of issues we had previously with number of pickers. LGTM, let me know if there some final touches you need to get done before I merge. Thanks

@Conni2461 Conni2461 merged commit 4a8ea77 into nvim-telescope:master Nov 26, 2020
@Conni2461 Conni2461 deleted the refactor_builtin branch November 26, 2020 07:17
kkharji pushed a commit that referenced this pull request Nov 27, 2020
…o builtin_runner_insert_issue

* 'master' of github.com:nvim-telescope/telescope.nvim:
  Refactor builtin (#287)
  fix: Use noremap when mapping. (#286)
  Fix the bug report template to suggest -u
  Add gitter tag
  feat: highlighter only
  Filter the completion of the command (#279)
  feat: Buffers rework (indicators and sorting) (#208)
  feat: v0.1 of extensions (#278)
  Register finder (#275)
  Various previewer fixes (#260)
  docs: fix builtin table formatting (#272)
  feat: Add highlights builtin (#267)
  Fixed minor typos (#271)
  Feat: Add filetypes builtin (#263)
  Fix: cwd detection of builtin.git_ (#264)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants