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

Telescope discards 'readonly' #252

Closed
Iron-E opened this issue Jun 24, 2022 · 12 comments · Fixed by #255
Closed

Telescope discards 'readonly' #252

Iron-E opened this issue Jun 24, 2022 · 12 comments · Fixed by #255
Labels
bug Something isn't working upstream Caused by code barbar.nvim depends on

Comments

@Iron-E
Copy link
Collaborator

Iron-E commented Jun 24, 2022

cc @anuvyklack, #251

Last merge commit broke Teleskope. Now opening any file from Teleskope opens it in insert mode. Even vim help files, which are readonly.

@Iron-E Iron-E added the bug Something isn't working label Jun 24, 2022
@Iron-E
Copy link
Collaborator Author

Iron-E commented Jun 24, 2022

See also: #251 (comment)

@Iron-E Iron-E added the need-feedback Further information from original poster was requested. label Jun 26, 2022
@anuvyklack
Copy link
Contributor

anuvyklack commented Jun 27, 2022

The very basic config:

use { 'nvim-telescope/telescope.nvim',
   config = function()
      require('telescope').setup()
   end
}

Then try:

:Telescope find_files

@againxx
Copy link

againxx commented Jun 28, 2022

Same issue here. Seems also related to neovim's neovim/neovim#18743

@Iron-E Iron-E removed the need-feedback Further information from original poster was requested. label Jun 28, 2022
@againxx
Copy link

againxx commented Jun 28, 2022

Changing the line

if not vim.bo[buffer].buflisted then

to

if not vim.api.nvim_buf_get_option(buffer, 'buflisted') then

works for me.
neovim/neovim#18743 uses the new API nvim_get_option_value(name, {buf = bufnr}), which seems to break some other plugins as well.
ggandor/lightspeed.nvim#152

@againxx
Copy link

againxx commented Jun 28, 2022

Sorry, I'm wrong. There are some other vim.bos in render needed to change.

local function render(update_names)

@Iron-E
Copy link
Collaborator Author

Iron-E commented Jun 28, 2022

@againxx so am I correct in understanding that this is a nightly-only issue?

Edit: yes, it seems 0.7 is unaffected. Thank you @anuvyklack for the repro

Edit 2: Considering that this usage of vim.bo was not listed in Following HEAD as a breaking change, and other plugins are also dealing with it, I'm inclined to suspect that this is an unintended consequence of that Neovim PR. I'm thinking we should follow up with them and make sure.

@romgrk what is your opinion?

@Iron-E Iron-E added the upstream Caused by code barbar.nvim depends on label Jun 28, 2022
@romgrk
Copy link
Owner

romgrk commented Jun 29, 2022

Sorry I might not follow correctly but why is this a barbar issue and not a telescope issue? Does the change from @againxx in barbar's codebase solves the issue? I've tried it on my side with latest neovim master, and it doesn't seem so.

For following up with upstream, I guess we should because neovim/neovim#19041 by @lewis6991 seems to have been meant to solve the neovim/neovim#18743 issue, if it's still present we should open an issue. Though I'm still not sure what's going on, comments appreciated.

@Iron-E
Copy link
Collaborator Author

Iron-E commented Jun 29, 2022

Our use of vim.bo is causing some behind-the-scenes wonkiness (due to recent changes in nightly) and making the readonly setting (possibly others too) drop when barbar reads/writes the buffer options using vim.bo. Telescope is just the vehicle for exposing this behavior.

If we were to go through and regex replace vim.bo with vim.api.nvim_buf_get_option, things would go back to normal. However, seeing as this is a nightly-only issue and there are follow-up PRs to address it in core, the question is whether we should fix it on our side, try to make sure it gets fixed everywhere, both, etc.

I guess we should because neovim/neovim#19041 by @lewis6991 seems to have been meant to solve the neovim/neovim#18743 issue, if it's still present we should open an issue.

That was my take as well.

Edit: Since that PR is merged now I can rebuild nightly and check again.

@romgrk
Copy link
Owner

romgrk commented Jun 29, 2022

I've done the change as it's easy and fixes the issue for those on master. And for the upstream issue, do we have an idea how to make a minimal reproducible config? I don't have much time to create a repro case and I don't use telescope, if someone wants to volunteer to please go ahead. Otherwise let's hear @lewis6991 thoughts, as the second PR on neovim seems to have been meant to fix this.

@Iron-E
Copy link
Collaborator Author

Iron-E commented Jun 29, 2022

It seems the latest Nightly fixes this issue, even still using vim.bo

@romgrk
Copy link
Owner

romgrk commented Jun 29, 2022

Oh ok, all good then. We can keep nvim_buf_get_option usage though, less magic is good.

edit:

It seems the latest Nightly fixes this issue, even still using vim.bo

And you have tested before the barbar update right?

@againxx
Copy link

againxx commented Jun 29, 2022

PR #255 works for me. However, the latest nightly build without it still keeps the cursor in insert mode after closing the Telescope window.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream Caused by code barbar.nvim depends on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants