parinfer

cfleming 2017-07-09T07:59:55.939166Z

@shaunlebron I’m definitely interested in updating the JVM version.

rgdelato 2017-07-09T19:06:39.668294Z

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)

shaunlebron 2017-07-09T19:48:38.878194Z

@rgdelato ha! i was just trying this today, lemme read what you have

shaunlebron 2017-07-09T19:49:28.882548Z

this looks great

shaunlebron 2017-07-09T19:49:58.885098Z

I was looking at buffer.onDidChange, but decided to use editor.onDidChangeText like you did

shaunlebron 2017-07-09T20:01:26.943370Z

@rgdelato I just tried it out, works well 👍

shaunlebron 2017-07-09T20:01:51.945630Z

i pushed a small cursorDx fix for indent mode last night - 2.5.1

rgdelato 2017-07-09T20:06:00.966531Z

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

shaunlebron 2017-07-09T20:07:37.974448Z

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

rgdelato 2017-07-09T20:10:00.986138Z

Doing that might still be a good idea.

shaunlebron 2017-07-09T20:10:34.989166Z

i’ll console.log to see what events are chaining

shaunlebron 2017-07-09T20:14:37.008525Z

might want to use ocall for these functions so we don’t need externs right?

shaunlebron 2017-07-09T20:18:09.025248Z

oh, and we can’t loop through the changes in a forEach, because the coordinates change on every iteration

shaunlebron 2017-07-09T20:19:00.029537Z

i think we should only respond to the first change for now, which handles the single cursor case

shaunlebron 2017-07-09T20:19:19.031107Z

i’ve only been able to reproduce multiple changes using multiple cursors

rgdelato 2017-07-09T20:25:46.061838Z

same for me, unless I'm using multiple cursors, I've only gotten 0 or 1 changes

rgdelato 2017-07-09T20:28:28.074827Z

also, it looks like there's only one cljsbuild config and it has :optimizations set to :simple

shaunlebron 2017-07-09T20:29:43.080777Z

ah, nevermind then

shaunlebron 2017-07-09T20:30:36.085534Z

@rgdelato it’s not firing a second onChangeText when the callback changes the text synchronously

shaunlebron 2017-07-09T20:30:56.087201Z

you were probably getting the infinite loop when debouncing

shaunlebron 2017-07-09T20:33:18.098766Z

minor edge case I can fix—cursorDx doesn’t shift comment lines

cfleming 2017-07-09T21:52:56.470415Z

@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.

cfleming 2017-07-09T21:53:17.472033Z

old-range-x (some-> change .-oldRange .-end .-column)
new-range-x (some-> change .-newRange .-end .-column)

cfleming 2017-07-09T21:54:05.475492Z

Is the cursorDx value really a delta between the range lengths, or should it actually represent the caret movement?

cfleming 2017-07-09T21:59:24.498831Z

(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).

cfleming 2017-07-09T22:07:40.538082Z

@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.

cfleming 2017-07-09T22:08:57.543684Z

Unless their changes are ordered by text offset rather than the order in which they occurred.

cfleming 2017-07-09T22:09:12.544730Z

Which doesn’t really make sense.

shaunlebron 2017-07-09T23:25:26.918509Z

@cfleming: cursorDx might be misdescribed then. We only want cursorDx to be 2. If it were 4, the second line would not shift correctly

cfleming 2017-07-09T23:25:59.921318Z

Right, so it’s really the difference between the old and new lengths.

shaunlebron 2017-07-09T23:26:27.923938Z

I suppose it doesn’t matter where the cursor is in the initial selection

cfleming 2017-07-09T23:27:06.927477Z

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.

shaunlebron 2017-07-09T23:27:07.927527Z

it should behave the same no matter what, since the cursor should be after the replacement selection

cfleming 2017-07-09T23:28:39.935838Z

@shaunlebron So with this new behaviour, is it your expectation that Paren mode is officially obsolete?

shaunlebron 2017-07-09T23:30:20.944905Z

it certainly feels obsolete so far

shaunlebron 2017-07-09T23:31:49.953132Z

let me try with multiline code

cfleming 2017-07-09T23:36:03.976520Z

Nice!

shaunlebron 2017-07-09T23:36:59.981613Z

yeah! I think ryan got it right by just subtracting the end column of the old from the new

cfleming 2017-07-09T23:39:08.993441Z

Right - I guess what you really want to know is how far following code should be indented/outdented

cfleming 2017-07-09T23:40:20.999675Z

This is good news for me, because I don’t have access to the actual caret when calculating this.

shaunlebron 2017-07-09T23:40:36.001282Z

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

cfleming 2017-07-09T23:41:26.005652Z

I’ll still need to think about the case of multiple changes, and try to work out how to apply those.

cfleming 2017-07-09T23:42:27.011470Z

I guess that the cursorDx applies to all code following a particular change inside its enclosing form, right?

cfleming 2017-07-09T23:43:09.015093Z

Does atom have a “wrap selection in parens” action? What happens if you select the if and then wrap it?

cfleming 2017-07-09T23:43:22.016250Z

In that case, is parinfer run once or twice?

shaunlebron 2017-07-09T23:44:48.024356Z

i’d just describe it as shifting all lines inside the expressions with open-parens after the cursor

cfleming 2017-07-09T23:45:27.027969Z

Well, it’s after the change, right? The cursor doesn’t actually matter.

shaunlebron 2017-07-09T23:45:49.029784Z

right, we’re using cursorDx because I haven’t finished support for the change option yet

cfleming 2017-07-09T23:46:02.030886Z

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.

shaunlebron 2017-07-09T23:46:04.031070Z

which will help with inline paren inference later

shaunlebron 2017-07-09T23:46:35.034037Z

i can install paredit plugin to see in atom

cfleming 2017-07-09T23:47:07.036976Z

Actually, is this new version available in the online editor yet? Or is there a test version somewhere?

shaunlebron 2017-07-09T23:48:25.044287Z

you can clone ryan’s fork: git@github.com:rgdelato/atom-parinfer.git

shaunlebron 2017-07-09T23:48:43.046078Z

and git checkout cursor-dx-poc

rgdelato 2017-07-09T23:48:50.046643Z

wrapping something in parens seems to only create one change in onDidTextChange

shaunlebron 2017-07-09T23:49:15.049144Z

using paredit?

cfleming 2017-07-09T23:49:21.049760Z

@rgdelato Right, so you receive an array of changes, right?

cfleming 2017-07-09T23:49:33.050803Z

Do you run parinfer after each one?

rgdelato 2017-07-09T23:49:44.051846Z

I just highlighted a block and pressed left paren (shift + 9), seems that wrapping behavior is built into Atom?

cfleming 2017-07-09T23:50:32.056719Z

Ok, so the wrapping is creating the wrapped text and replacing the selection with it, looks like.

cfleming 2017-07-09T23:50:44.058003Z

Rather than just inserting the two parens individually.

cfleming 2017-07-09T23:51:36.062931Z

Is there another way we could create a transaction with multiple changes?

cfleming 2017-07-09T23:52:50.069982Z

Can you rename elements in Atom?

rgdelato 2017-07-09T23:53:12.072030Z

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

cfleming 2017-07-09T23:53:36.074412Z

I won’t be able to do that in Cursive, at least.

rgdelato 2017-07-09T23:53:48.075396Z

> 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

cfleming 2017-07-09T23:54:20.078403Z

There you go - looks like the paredit support works like the Cursive one would.

cfleming 2017-07-09T23:54:47.080936Z

I think you’ll have to handle that.

shaunlebron 2017-07-09T23:56:10.088493Z

the renaming case will be problematic too

cfleming 2017-07-09T23:56:50.092484Z

Yes.