Open pitalig opened 4 years ago
Yes, it would be great if this worked! PR welcome.
+1
+1
I would be honored to take this on.
Any pointers or tips before I get started?
Awesome news, @rayat-amperity ! Please consider touching base in the #calva
channel on the Clojurians slack. I'd love to give you a guided tour in the relevant parts of the code base.
(full disclosure, I am not an honorable man, just one who loves multi-cursor).
(btw big fan of your work and talks @PEZ, and my coworkers have spoken very highly of you personally!!!!!)
I'm blushing here! Please tell your colleagues that I thoroughly enjoyed meeting them and have that chat about Calva. Huge thanks for relaying this. You just made my day. ❤️
I think you should be able to join Clojurians via this link: http://clojurians.net/
Good idea with recording such a guided tour. I touch a bit on some of the relevant parts of this, in my Seajure presentation, which you might have heard your colleagues mention. Here's a deep link: https://youtu.be/CcPxHGm0JWo?t=6155
<3
Thanks!
Awesome, just saw that portion. Funny to see my coworkers like that, never had that happen before, man the clj community is intimate!
So I've been toying around in the codebase and have a decent sense of what needs to be done to start with. Preliminary observations, notes or questions:
The various models that wrap the vscode document apis make certain assumptions, the most notable of which for this issue's sake are that editors/documents have a single selection or cursor location at a time.
TextEditor.selection
instead of TextEditor.selections
(or TextEditor.selections[0]
) in the implementers of EditableDocument (namely MirrorDocument)..selectionLeft/Right
properties whose semantics change or whose utility diminish if we try to keep multiple cursors in mind.
return doc.selections.map(s => ({ anchor: s.anchor, active: s.active }))
vs return ({ anchor: doc.selectionLeft, active: doc.selectionRight })
, so the differences could be subtle, or more than that. I'll have a better sense myself as I dive into this more. I think that generally it would be good to start with cleaning away all the places where selectionLeft
is used, where selection.active
should have been used instead. I don't know, without investigating, wether there are any cases where we actually need Left/Right semantics. It's probably rare, if at all. In those cases we can probably use functions or dynamic properties that derive from anchor/active.
A PR that only does this. Or only does this at the places we deem perfectly safe, would be a good start, and a huge improvement of the codebase, regardless of multi-cursors. Please also touch base on the slack.
Before I just read your comment, I had gotten Expand Selection / Shrink Selection to work! At least it looks like it from messing around in the test folder. Other untouched commands work as they did previously. I'll open a draft PR just so you can maybe provide feedback, or if you're simply curious, in the meantime.
Once that's open I'll switch over to work on your suggestion above first, and I'll rebase my multi cursor work upon that.
I saw code for expand selection that looked like it might work. 😄 This is so exciting!
Please check out the HELP announcement here with testing instructions.
You can download the testing version of the extension in the PR and let us know what it's like. It'll come out faster the more you help out!
When I have multiple cursors, paredit commands just affects the first one. This gif shows an example when I try to slurp 3 forms, but it just affects the first one. The expected result was: