13 Sep 2024 |
dundargoc | Sure | 14:51:07 |
gpanders | I completely missed https://github.com/neovim/neovim/pull/30313 while it was happening but THANK YOU Riley Bruins for pushing that through | 15:37:32 |
gpanders | that has long annoyed me (and I'm pleased to see it was an uncontroversial change) | 15:38:23 |
Mathias | huh, how is one supposed to adapt plugins to that if get_parser results in a warning if the parser is missing no matter what? | 15:49:45 |
gpanders | switch to _get_parser temporarily, then switch back to get_parser (not ideal, but it works) | 15:58:49 |
gpanders | * switch to _get_parser temporarily, then switch back to get_parser after 0.12 (not ideal, but it works) | 15:58:54 |
clason | In reply to @mfussenegger:matrix.org huh, how is one supposed to adapt plugins to that if get_parser results in a warning if the parser is missing no matter what? Sorry, I don't quite get that. | 16:03:22 |
clason | The warning should not trigger the pcall ? | 16:03:38 |
clason | (At least it shouldn't) | 16:03:48 |
clason | I didn't get to review the final state before it got merged, so I'm not sure what the actual behavior is | 16:04:47 |
Mathias | Maybe I read the diff wrong, but I think now get_parser will emit a warning whenever you call it on a parser that doesn't exist | 16:06:59 |
clason | yes, as it should? | 16:07:36 |
Mathias | yes, but how are plugins supposed to migrate so that users don't get a warning? | 16:07:55 |
clason | before, it triggered an error | 16:07:57 |
clason | ah, I see what you mean | 16:08:08 |
clason | (error got slurped by pcall) | 16:08:44 |
clason | hmmm, that's a tough one (and one of the reasons the pcall model is so bad): do you want notification, or not? | 16:09:19 |
Mathias | I don't want notification because I have a fallback logic if the parser doesn't exist | 16:09:45 |
clason | Yes, you don't | 16:09:56 |
clason | But others may have relied on the explicit error message | 16:10:08 |
clason | (not defending the current behavior; I agree that it's suboptimal -- just trying to figure out what the right deprecation approach is here) | 16:11:32 |
lewis6991 | I'm pretty sure in this case we don't want the error. It's up to the application to deal with that. | 16:11:41 |
clason | because pcall needs to die, one way or another | 16:11:46 |
Mathias | Could do it like iter_matches and add a parameter? | 16:12:12 |
clason | In reply to @lewis6991:matrix.org I'm pretty sure in this case we don't want the error. It's up to the application to deal with that. No, I mean specifically the deprecation/migration strategy | 16:12:17 |
Mathias | Otherwise:
local ok, parser
if vim.treesitter._get_parser then
parser = vim.treesitter._get_parser(bufnr)
ok = parser ~= nil
else
ok, parser = pcall(vim.treesitter.get_parser, bufnr)
end
would do the trick, but that means using private method
| 16:12:33 |
clason | the new behavior is correct and should become the default | 16:12:54 |
clason | In reply to @mfussenegger:matrix.org Could do it like iter_matches and add a parameter? Maybe. I'd like to get an idea how the function is used, though, before we implement a strategy | 16:13:44 |
clason | (learn from mistakes and whatnot) | 16:13:58 |
clason | Thinking about it, the current deprecation approach is the worst of both worlds | 16:14:26 |