@shaunlebron I’m definitely interested in updating the JVM version.
Also currently playing with a proof-of-concept to get cursorDx working in Atom: https://github.com/rgdelato/atom-parinfer/commit/ab2c8c643900de8c8fb309393aa4d5b6d275d027 ...I definitely need to test it a bit more, but it looks like something in this ballpark could potentially work? ("It works on my machine." :P)
@rgdelato ha! i was just trying this today, lemme read what you have
this looks great
I was looking at buffer.onDidChange, but decided to use editor.onDidChangeText like you did
@rgdelato I just tried it out, works well 👍
i pushed a small cursorDx fix for indent mode last night - 2.5.1
Good to hear that it worked for you as well! When I was very first trying it out, I was getting an infinite loop of onChange -> run Parinfer -> setText -> setText triggers onChange -> etc ...and honestly, I'm not quite sure what's preventing that infinite loop from happening in the current version of the code
since the apply-parinfer will set the full text of one or more top-level forms, I was going to store that and check on future change events for reactions to that value and break out
Doing that might still be a good idea.
i’ll console.log to see what events are chaining
might want to use ocall for these functions so we don’t need externs right?
oh, and we can’t loop through the changes
in a forEach, because the coordinates change on every iteration
i think we should only respond to the first change for now, which handles the single cursor case
i’ve only been able to reproduce multiple changes using multiple cursors
same for me, unless I'm using multiple cursors, I've only gotten 0 or 1 changes
also, it looks like there's only one cljsbuild config and it has :optimizations
set to :simple
ah, nevermind then
@rgdelato it’s not firing a second onChangeText when the callback changes the text synchronously
you were probably getting the infinite loop when debouncing
minor edge case I can fix—cursorDx doesn’t shift comment lines
@rgdelato @shaunlebron Looks like Atom is assuming that the caret is at the end of the changed range - I don’t think that’s always true.
old-range-x (some-> change .-oldRange .-end .-column)
new-range-x (some-> change .-newRange .-end .-column)
Is the cursorDx value really a delta between the range lengths, or should it actually represent the caret movement?
(example of when that wouldn’t be true: I have the caret at the end of an identifier. I use shift-left arrow to select to the start of the identifier, so the identifier is selected but the caret is at the beginning. Then I paste some replacement text. cursorDx should be the length of the replacement text, not the difference between the old and new text).
@shaunlebron I saw your github comment, thanks. I still don’t quite understand what their oldRange represents, i.e. I don’t see how their code works. I’ll have to dig into the atom code to see if I can figure out how they’re calculating that.
Unless their changes are ordered by text offset rather than the order in which they occurred.
Which doesn’t really make sense.
@cfleming: cursorDx might be misdescribed then. We only want cursorDx to be 2. If it were 4, the second line would not shift correctly
Right, so it’s really the difference between the old and new lengths.
I suppose it doesn’t matter where the cursor is in the initial selection
Hmm, thinking about it, that’s not right either. It’s the difference between the final column of the old and new. If you paste multi-line code you can’t use the lengths.
it should behave the same no matter what, since the cursor should be after the replacement selection
@shaunlebron So with this new behaviour, is it your expectation that Paren mode is officially obsolete?
it certainly feels obsolete so far
let me try with multiline code
Nice!
yeah! I think ryan got it right by just subtracting the end column of the old from the new
Right - I guess what you really want to know is how far following code should be indented/outdented
This is good news for me, because I don’t have access to the actual caret when calculating this.
yeah, and rather than specifying the full change, we use the cursor point as reference for the amount of shift that should happen to the expressions starting after it
I’ll still need to think about the case of multiple changes, and try to work out how to apply those.
I guess that the cursorDx applies to all code following a particular change inside its enclosing form, right?
Does atom have a “wrap selection in parens” action? What happens if you select the if and then wrap it?
In that case, is parinfer run once or twice?
i’d just describe it as shifting all lines inside the expressions with open-parens after the cursor
Well, it’s after the change, right? The cursor doesn’t actually matter.
right, we’re using cursorDx because I haven’t finished support for the change
option yet
I’m thinking about the case where there are two changes (i.e. wrapping in parens). In that case, it seems like the two dx’es should be summed after the second change.
which will help with inline paren inference later
i can install paredit plugin to see in atom
Actually, is this new version available in the online editor yet? Or is there a test version somewhere?
you can clone ryan’s fork: git@github.com:rgdelato/atom-parinfer.git
and git checkout cursor-dx-poc
wrapping something in parens seems to only create one change in onDidTextChange
using paredit?
@rgdelato Right, so you receive an array of changes, right?
Do you run parinfer after each one?
I just highlighted a block and pressed left paren (shift + 9), seems that wrapping behavior is built into Atom?
Ok, so the wrapping is creating the wrapped text and replacing the selection with it, looks like.
Rather than just inserting the two parens individually.
Is there another way we could create a transaction with multiple changes?
Can you rename elements in Atom?
I've only been able to make it occur with multiple cursors/selections. Shaun was saying earlier that we should only take the first change in the changes array
I won’t be able to do that in Cursive, at least.
> oh, and we can’t loop through the changes
in a forEach, because the coordinates change on every iteration
> [1:19]
> i think we should only respond to the first change for now, which handles the single cursor case
There you go - looks like the paredit support works like the Cursive one would.
I think you’ll have to handle that.
the renaming case will be problematic too
Yes.