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

Use tree-sitter for deciding on adding end #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jemmyw
Copy link

@jemmyw jemmyw commented Sep 21, 2021

I'm opening this PR as a discussion rather than actually looking for this to be merged.

I got frustrated with endwise adding end all the time when I'm writing examples in comments. I thought there would be a simple fix, but I got to thinking about other times I've been using it where it doesn't quite do the right thing.

In this modified version I've swapped out the regex parsing with tree-sitter. It should be possible to then extend this code to do something similar to the current version, but more reliably using the tree-sitter data structure. However, it actually works pretty well with the stupidly simple algorithm of "is the program invalid or would it be valid if I add end to the next line?"

It would also solve the problem of adding other languages as per the TODO as it's just a case of bundling the tree-sitter module.

endwise-2021-09-21_12.32.24.webm.mp4

@vfonic
Copy link

vfonic commented May 27, 2022

Hey @jemmyw!

This PR change is a great idea!

I tested it out and it seems to work pretty well!
I couldn't make it work in files containing endless methods or omitting keyword arguments, but at least it stopped adding end statements after commented-out code.

Endless methods is a new feature since Ruby 3.0 and omitting keyword arguments is added in Ruby 3.1. Could it be that tree-sitter doesn't support these Ruby versions yet?

Thanks!

@vfonic
Copy link

vfonic commented May 27, 2022

Yes, the issue is at tree-sitter-ruby's end. I've posted an issue in their repo:
tree-sitter/tree-sitter-ruby#219

@alfuken
Copy link

alfuken commented Jul 24, 2022

@jemmyw can you, please, release your version on Marketplace? Or release a compiled drop-in replacement with your changes? Current version, if used with GitLens (or at least that's what the extension bisect says), creates immense lag on each "enter" keypress for me, up to 1s, and I was hoping that your version would behave better in this matter.

@jemmyw
Copy link
Author

jemmyw commented Jul 24, 2022

Hi @alfuken I haven't had a chance to go back to this and make any improvements so it might just act pretty rough. Here is a link to a release where you can download the vsix file and install that into vscode https://github.com/jemmyw/vscode-endwise/releases/tag/tree-sitter-0.0.1

@alfuken
Copy link

alfuken commented Jul 24, 2022

@jemmyw thanks a lot! 👍

@ndbroadbent
Copy link

This is pretty cool! @alfuken and @jemmyw, are you still using this branch, and is it working well for you? I'm tempted to switch if it is better than the current implementation. But I actually haven't run across too many examples where it's doing the wrong thing.

Are you talking about =begin and =end comments? I tried it out and I feel like it's still useful to add end automatically in those. Or other kinds of comments?

@vfonic
Copy link

vfonic commented Sep 30, 2022

I've made a fork which adds a new line first and then checks whether the end keyword is needed or not.

For example:

Before:

def asd|end => | being the cursor position

If you press ENTER here, it will change the code to something like this:

def asd
  |end
end

After the fix:

def asd
|end

It's not perfect, but better than inserting an extra end keyword.

https://github.com/vfonic/vscode-endsitter

@jemmyw feel free to pull in this commit into your extension.

@ndbroadbent
Copy link

Nice! I guess this might just be an issue with the new endsitter implementation, since the case you described isn't happening for the original vscode-endwise extension.

Screen.Recording.2022-10-01.at.11.51.51.AM.mov

Would be kind of cool if it could detect this case though, and add a new line + move the cursor up.

@vfonic I'll try out your version today and see how it goes.

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.

4 participants