cursorless
cursorless copied to clipboard
cursorless-dev
Migrated surrounding pair scope handler
Checklist
- [x] I have added tests
- [/] I have updated the docs and cheatsheet
- [ ] I have not broken the cheatsheet
- [ ] Add
@disqualifyDelimiterfor all languages - [ ] Add plural form of surrounding pair scopes.py
I would argue that this is not a matching delimiter pair at all. It is the scope type string which we have no spoken form for at the moment. https://github.com/cursorless-dev/cursorless/blob/766864d9fa979124283b236bbb4738204a13fe45/data/fixtures/recorded/languages/ruby/changeString2.yml
This is really interesting
"if the file is over 30MB or over 300k lines, we consider it to be a very large file and proceed to turn off very many features, including tokenization, word wrapping, indent guides, anything that would consume additional memory, etc." https://github.com/Microsoft/vscode/issues/32118#issuecomment-320952246
That means that Cursorless can never encounter a file with over three hundred thousand lines. I just did a test with the surrounding pair regex on a file with just below three hundred thousand lines and that clocked in at 274ms. I would say that is a bit on the high side so we might want to do some windowing, but we definitely don't need to be as careful as we was before. Doing it at tens of thousand lines at the time probably is fine.
is there a reason this is still marked as draft? Looks like there's some unchecked boxes in description too
Yes the checkboxes in the description are not done. I wasn't sure this implementation was going to get your approval so I didn't go further originally.
Got it. So would it make sense to knock those out now that we're aligned on the high-level direction or did you want me to take a look first?
Please have a look first. Some of the implementation details I'm not sure off myself. The single line strings is one thing that come to mind.
Ok I've now fully reviewed this PR. I made a bunch of changes. I think what remains are:
- [x] Review my changes to make sure you're happy
- [x] Finish the checkboxes in the description
- [x] Address my remaining unresolved comments
Getting really close tho! Very excited to have this one
Ready for review. I have added @disqualifyDelimiter to all the languages that have boolean expressions(using </>) to my knowledge.
@pokey I have now gone through all the grammar json files and added all the operators I can find that clashes. I might have missed something; some of these files are quite long and complicated, but I done my best.
wow that disqualify stuff looks like it was a lot of work. Nice job. We should prob have some negative tests, ie that we're not disqualifying delimiters that we do want, but I think we have some tests for those and this PR is growing long in the tooth so I'd be happy to leave those for now.
As discussed, let's drive this for a couple days and then ship it
It was definitely some effort. We have a few tests. If anything comes to mind please record a few. Otherwise I would like to just get this in at the stage. I've been running it for this entire day and except for the bug that was fixed earlier it has been working fine.
I wonder if we should make iteration scope be "line". I think being able to say "first string" / "last string" will be really useful
I wonder if we should make iteration scope be "line". I think being able to say "first string" / "last string" will be really useful
Why not?