symex.el icon indicating copy to clipboard operation
symex.el copied to clipboard

Add support for non-Lisp languages by using tree-sitter

Open countvajhula opened this issue 4 years ago • 11 comments

Summary of Changes

Use tree-sitter for non-lisps.

Remaining Work

✨ See the Kanban Board. 📒

Older completed items:

  • [x] Groundwork and basic movements (PR: #32)
  • [x] Fix leap-branch (see this comment) (PR: #42)
  • [x] Add basic transformations (e.g. d, c, y, p) (PR: #47)

countvajhula avatar Nov 04 '21 19:11 countvajhula

Hi @polaris64 , now that RacketCon-related activities are abating, I should have more time to dedicate to this starting next week. I'm thinking I should look into the leap branch issue first since that may be more internals-facing, unless you're working on that already?

countvajhula avatar Nov 13 '21 18:11 countvajhula

Hi @countvajhula, I haven't had any time to work on this recently I'm afraid, so please feel free to go ahead! Fixing the leap branch issue does indeed sound like a good start as I have a feeling that fixing the underlying issue here will (I hope) fix the other movements.

polaris64 avatar Nov 14 '21 14:11 polaris64

Ok, no worries @polaris64 ! I'll take a look soon and will keep you in the loop.

countvajhula avatar Nov 15 '21 00:11 countvajhula

Just FYI: I've rebased this integration branch to avoid merge conflicts later on. If you have an existing version of this branch checked out, you would need to delete it and re-pull before starting any fresh branches on it. Happy to help if you run into any difficulties with this.

countvajhula avatar Jan 07 '22 20:01 countvajhula

@polaris64 OK all merged and this branch has been rebased to master 👍 Happy to collaborate on the transformations PR, just let me know!

countvajhula avatar Jun 16 '22 06:06 countvajhula

@countvajhula Excellent, I've rebased my fork and everything looks good!

I've started some very basic work into deleting forward/backward and so far it seems to be working well, even including count prefix arguments. I've taken your lead from symex-primitives.el and I'm calling my own routines based on the value of tree-sitter-mode. It'll probably be worth splitting the current transformations into something like symex-transformations-lisp for tidiness as you've done with symex-primitives.

I'm excited to get this next stage done as I can already see the benefit while testing. Combining this with the PR which adds support for evil-repeat should really make take this TS integration to the next level!

polaris64 avatar Jun 16 '22 13:06 polaris64

That's great! And sounds good to me.

countvajhula avatar Jun 16 '22 16:06 countvajhula

super excited for this, will download and mess with transformations once they are available!

tommy-mor avatar Jun 16 '22 17:06 tommy-mor

@countvajhula, @tommy-mor: please see this PR for the draft TS transformation integration

polaris64 avatar Jun 20 '22 12:06 polaris64

I've migrated a few of the outstanding transformations from the ts-transformations PR into the present PR description and also removed a few that aren't very common / just nice-to-have.

Now that @polaris64 's transformations PR has been merged, I think we've basically achieved the necessary architecture of the codebase to have a common interface that abstracts over the primitive (i.e. Lisp and Tree sitter) operations.

The remaining issues prior to merging consist mostly of handling edge cases for a clean UX, and supporting remaining commonly-used transformations, and testing + fixing any bugs that may be discovered along the way (anyone following along and using this branch -- git pull to get the latest and try it out!)

At this point, the remaining items in the PR description should be bite-sized and bounded, and it should be easier for anyone to pick one to work on whenever we are able to. For anyone reading this, if you feel inspired to tackle any particular task, just start a quick draft PR with whatever initial work you may have done, at the earliest possible stage (e.g. after you've done a single commit). That way, other contributors can see that someone is working on that task and can work on something else that they may be interested in.

Alongside that push to the finish line, given the nature of where we are and what remains, I've also added an investigative task to play around with tree-edit and try to use it in the implementation of a single transformation like symex-delete. Depending on what we learn from this initial POC, it will help us plan for when an integration would make sense (for instance, would it be best (a) after releasing the current work, or (b) as part of the current work, if it turns out that it will help us avoid writing hacks for corner cases and other ad hoc code), and also what such an integration might look like (e.g. are any changes required in either symex or tree-edit that would aid a seamless integration?). This is independent of and doesn't block any of the other tasks. (FYI @ethanleba)

countvajhula avatar Aug 30 '22 23:08 countvajhula

Actually, let's try out GitHub projects to track the remaining items, that might be easier / better.

I've created a Kanban board at Symex TS Integration and added you as an admin @polaris64 , and also given you write access @tommy-mor and also @anonimitoraf [edit]. It has all of the items that were in the PR description above and I've added some basic descriptions to them as well (can make them more detailed if we feel that would be helpful).

We could follow this process:

  • If you're working on a task, either start a PR or (if you have write permissions) move the corresponding ticket to the "in progress" column and add yourself as the assignee. Otherwise, I can add you as the assignee if you start a PR.
  • If you find any issues while testing this branch, create a new ticket on the board and add a description. If you don't have write access, just post a comment here with the bug you found and I'll add it to the board and also give you write access.

(btw no pressure -- any contribution is appreciated!)

How does that sound?

countvajhula avatar Aug 31 '22 00:08 countvajhula

This looks epic. Will try to test at some point. Quick question: does this use Emacs 29 in-built treesitter or the older package-with-external-program style?

jdtsmith avatar Jan 10 '23 18:01 jdtsmith

@jdtsmith It uses the older library. I'm not too familiar with the new built-in tree sitter functionality, but would adopting it mean dropping support for < Emacs 29 users? If so, I think it would probably be best to continue with the existing older library and then upgrade to the built-in functionality once there has been enough time for users to adopt Emacs 29+.

Luckily, a big part of the tree-sitter effort for Symex so far has been about abstracting different layers of operation. So I don't anticipate changing the tree-sitter provider to cause difficulties down the line.

countvajhula avatar Jan 10 '23 22:01 countvajhula

Ok thanks. Yeah you'd need to start with 29. Given that symex-ts hasn't been released maybe backward compatibility isn't as big a problem. In 29 ts runs in process so is faster. I think the API's are quite different too.

Will be watching this space!

jdtsmith avatar Jan 10 '23 23:01 jdtsmith

Hi, sorry that I'm a bit late to the conversation, I'm currently on paternity leave.

I actually started work on adding support for the newly built-in tree sitter library in Emacs 29. The two interfaces are actually quite similar so adding support wasn't too tricky. The main difference comes when comparing trees after a mutation, so that'll need a slightly more in-depth change. That only affects one macro however so it shouldn't be too bad.

The way I have it working so far is that Symex will check for built-in support during initialisation. It'll then set up aliases (or define proxy functions) to provide a Symex-specific abstraction to whichever underlying tree sitter library it needs to use.

If you're interested then my fork of this repo contains a scratch branch with my changes so far. Once I've finished then I plan to merge this with the this feature branch.

Sorry for the lack of relevant links, I'm currently away from my desk and having to reply via email.

On 11 January 2023 00:28:02 CET, JD Smith @.***> wrote:

Ok thanks. Yeah you'd need to start with 29. Given that symex-ts hasn't been released maybe backward compatibility isn't as big a problem. In 29 ts runs in process so is faster. I think the API's are quite different too.

Will be watching this space!

-- Reply to this email directly or view it on GitHub: https://github.com/drym-org/symex.el/pull/33#issuecomment-1378029631 You are receiving this because you were mentioned.

Message ID: @.***>

polaris64 avatar Jan 11 '23 11:01 polaris64

@polaris64 Perfect, that sounds like a good way to do it! Btw, congrats on the new arrivals 😃

countvajhula avatar Jan 11 '23 19:01 countvajhula

Hey everyone, I hope you're all doing great! Strap yourselves in for this one...

When I decided to use Symex as one of the initial projects being appraised at the upcoming ABE Congress, I did it for two main reasons:

  1. As the original author, I had the authority to do it. Being in the position of sharing donations to "my"[1] project with others is a much easier sell than asking someone else to do so, especially when they have no reason to trust me or the DIA process.
  2. Since Symex has nontrivial contributions from many people, the result of the congress would mean that many people would become eligible to receive payments from Symex, and not just me.

But here's the thing. A requirement for the purposes of DIA is that projects are appraised in terms of work that's already being used, i.e. in the main branch. One of the main nontrivial contributions I had in mind with Symex was the tree-sitter work. This branch, obviously, is as yet unmerged. If the outcome of the DIA congress is that I am entitled to almost all of the revenues from Symex, that would be a boring outcome and wouldn't prove the model, i.e. defeating one of the main reasons I chose this project.

So here's what I propose. Let's merge this branch in the next couple of days, i.e. before Saturday! 😂

Is this a very irresponsible thing to do, when we haven't given sufficient time for people to review it? Yes. But is it OK to do it? I think so, because I (and I think others) have been using this branch for some time and there have been no reported regressions on the existing functionality. That is, merging will "do no harm," and we can certainly consider the tree-sitter functionality to be "alpha." This will not constitute a new "2.0" release of Symex just yet, and we will wait to do that until we have had more time to add the remaining functionality and thoroughly review and test the code (and there will be no announcement until that stage). I propose that, since merging into main is a big step, that we set ourselves a deadline of March 20 to do the 2.0 release.

What do y'all think?

[1] Of course, it isn't "mine." Symex is owned by no one!!! 🐺

countvajhula avatar Jan 11 '23 23:01 countvajhula

haven't contributed much to branch, but have been using it. I approve 👍

tommy-mor avatar Jan 11 '23 23:01 tommy-mor

The branch has been rebased and is ready to be merged. Any review or testing at a high level would be appreciated of course, and unless there are any objections, I'll aim to merge this in tomorrow.

countvajhula avatar Jan 13 '23 00:01 countvajhula

Okay, here we go! 🏇

countvajhula avatar Jan 14 '23 00:01 countvajhula