elijah-potter / harper

The Grammar Checker for Developers
https://writewithharper.com
Apache License 2.0
749 stars 23 forks source link

VS Code Support #116

Open mcecode opened 3 weeks ago

mcecode commented 3 weeks ago

Hi! Previously, I've been using Grammarly for VS Code as my grammar checker because I couldn't find a better alternative. Recently, it stopped working and I was so happy that I was able to find Harper.

I saw that you created a VS Code extension in packages, but haven't published it yet. With some changes, I was able to package it using vsce and install it locally. It works great already, gives good feedback and I'm able to do quick fixes.

One suggestion I have is to either bundle it with the latest version of harper-ls when publishing it or to set it so that it automatically downloads, updates to, and uses the latest version of harper-ls from GitHub releases. I just thought that would make it more convenient for users since they don't have to download it manually themselves. Users who want to opt-out of that can just provide the path to their harper-ls installation in the settings and the extension would use that instead.

Something I did notice from the logs though is that even if it says it can't load the dictionary file, it is loading it and is applying the list of words I put there. Here's a screenshot of the error:

harper-error

All in all, this is just my general feedback on the extension and a thanks for creating this project. I'd be willing to help with getting the VS Code extension up and running. I'm sure previous users of Grammarly for VS Code like me would be happy if this got published. I can upstream the changes I made so that it's packageable by vsce or I can help with implementing my suggestion above if you're down with that.

elijah-potter commented 3 weeks ago

Wow, I'm seriously impressed you were able to do all that! It's great to hear it's going so smoothly.

I'm curious what you've changed. Feel free to open a pull request and we'll get vsce hooked into CI.

One suggestion I have is to either bundle it with the latest version of harper-ls when publishing it or to set it so that it automatically downloads, updates to, and uses the latest version of harper-ls from GitHub releases

I've considered both of those options, but I won't have time in the short term to get that implemented myself. If you're interested, feel free to toy around. If you end up getting something working, open a new pull request (or add it onto the other one) and we'll get the extension published.

The "errors" you've found in the logs aren't super self explanatory. They aren't actually errors, just a product of how harper-ls manages those dictionaries. I'll remove those logs so as to not confuse anyone else.

mcecode commented 3 weeks ago

I'm curious what you've changed.

Not much tbh, mostly just adding a publisher field in the package.json file and fixing where main was pointing.

Feel free to open a pull request

It's up in #117. I've also added some documentation and some files like LICENSE that prevents vsce from throwing warnings.

I've considered both of those options, but I won't have time in the short term to get that implemented myself. If you're interested, feel free to toy around.

I'll try to figure out how to do this and get back to you here when I have something working. I think I'll opt to try to implement the latter that downloads harper-ls from GitHub releases since I don't think the extension needs to change every release of harper-ls, only if harper-ls has a new feature that the extension needs to support like a new config.

They aren't actually errors, just a product of how harper-ls manages those dictionaries. I'll remove those logs so as to not confuse anyone else.

Good to know. I was wondering if the errors were because I was doing something wrong.

mcecode commented 6 days ago

Hi, sorry it took me a while to give an update on this. I actually have something that roughly works, but as I was working on it, I was checking out related VS Code issues and how other extensions handle binary dependencies to see if I'm heading in the right direction. I found that its kind of a split, some extensions bundle binaries while others download them on activate. Those that do download and manage the binaries themselves tend to have a complex dependency story like the C# extension, need/want to manage their dependencies with language-specific tooling like the Go extension, or was created before publishing platform-specific extensions was possible so they had to roll their own solution like shell-format. That said, our situation doesn't fall in any of these three so I'm now leaning more to bundling the harper-ls binary and publishing per platform rather than rolling out some sort of package managing solution.

For one, VS Code maintainers seem to prefer self-contained extensions. Additionally, package managing would require more maintenance overtime as we'd be duplicating a lot of what VS Code already does when it manages extensions like handling proxies, unzipping/untarring archives, gracefully handling download failures, etc. Further, publishing one extension for all platforms means that some users will always get an error when trying to download harper-ls for their platform since VS Code supports more platforms than harper-ls.

Overall, when I thought about it, bundling the binary would lead to more predictable outcomes. So, I'll try to get back to you again in a week or two, hopefully with a PR ready. Feel free to chime in and tell me if I'm heading in the wrong direction or if I'm missing something.

elijah-potter commented 5 days ago

Thanks for the update, and your work so far. What you've described sounds perfect. Let me know if there's anything I can do to help.