search

dmmulroy / ts-error-translator.nvim

MIT License
226 stars 5 forks source link

fix!: reuse existing handler rather than clobbering it #18

Closed mehalter closed 6 months ago

mehalter commented 6 months ago

Hey! This is a great idea for a plugin! I was taking a look at the source code and noticed that you are essentially clobbering any previously set lsp handler for publishing diagnostics. This makes it so that it can be an addition.

I also did a bit of refactoring for performance and to simplify the code. Tables in lua are passed by reference so rather than making new tables and returning them it can all be done in place to save computation and space overhead!

Let me know what you think as this does change the API a bit which I added to the README to help users who are building it into their own lsp handler.

Oh lastly, I noticed that it is only replacing the message if parameters are found, I removed that check since a lot of the error messages don't have parameters (like 1002 for unclosed strings)

Also while I was here I added vtsls support (closes #16)

dmmulroy commented 6 months ago

This is awesome! Thank you for the contribution - I'll take a look after work today.

mehalter commented 6 months ago

Awesome thanks! Definitely let me know what you think of the removal of the number of params check. I didn't do exhaustive testing so I just want to make sure that doesn't actually introduce bugs that you were avoiding before, but in some basic testing it looks like it works just fine.

No problem! I was just looking to add it to the AstroNvim plugin "marketplace" for our users!

dmmulroy commented 6 months ago

Looks good to me! Thanks for the clean up and improvements