Fork me on GitHub
#parinfer
<
2016-02-15
>
cfleming04:02:17

So I have very basic parinfer working now. Here are some problems I’ve found:

cfleming04:02:40

Paren mode lines up subforms of lists with a one-space indent:

cfleming04:02:57

(defn foo 
"I don't do a whole lot."
[x]
(println x "Hello, World!"))

cfleming04:02:39

(defn foo 
 "I don't do a whole lot."
 [x]
 (println x "Hello, World!"))

cfleming04:02:52

Those lines should be indented by two spaces.

cfleming04:02:15

The problem is, thinking about it, that line indentation requires semantic knowledge.

cfleming04:02:16

Here they should be indented by 2 spaces, but in the following case they should be aligned under the first param:

cfleming04:02:37

(println test 
         :test 
         "test")

cfleming04:02:21

Another problem: how should undo work? Due to debouncing, parinfer is executed asynchronously some time after the actual change. This means that the parinfer change has a different undo event, and an undo action will only undo the parinfer correction, not the original edit. This will leave the doc in a bad state and confuse the user.

cfleming04:02:55

Another problem: The caret often ends up in the wrong location after the parinfer correction. There’s no tracking of caret location in the algorithm IIRC, this is very surprising behaviour.

cfleming04:02:20

Another problem: Indent mode does not correct indentation when parens are moved around.

cfleming04:02:27

Here’s an example of the last two problems:

cfleming04:02:53

(let [x 10]<caret>
  (when true))

cfleming04:02:08

I end up with:

cfleming04:02:37

<caret>
(let [x 10])
  (when true)

cfleming04:02:13

Here, the caret is in the wrong place (it should be after the newly-inserted ) and the indentation of the (when true) is wrong.

cfleming04:02:36

If these are due to something I’m doing wrong in my implementation, let me know - likewise if they’re real problems that deserve an issue in the tracker.

cfleming04:02:47

I’m also interested to know if other implementations suffer from these problems.

cfleming04:02:39

I believe I can fix the caret movement problem - I’m planning to build up a list of changes required to the document and apply them individually rather than wholesale replacing the text. This will work better with anything else that’s driven by document changes, and also has the nice side effect of (hopefully) ensuring that the caret ends up in the right place.

cfleming04:02:26

Any opinions on whether it’s worth implementing this with only indent mode support? It seems that that’s what everyone uses anyway - are there situations where paren mode is actually required, or does it just provide a pleasing sense of mathematical symmetry? simple_smile

cfleming05:02:02

Here’s something else that’s a bit strange:

cfleming05:02:20

(let [x 10]<caret>)

cfleming05:02:28

When I press enter here, I get:

cfleming05:02:30

(let [x 10])
  <caret>

cfleming05:02:53

This looks strange to me, but perhaps it makes sense since if I then type x I get:

cfleming05:02:16

(let [x 10]
  x<caret>)

shaunlebron11:02:38

sweet! congrats on getting basic parinfer working in cursive simple_smile

shaunlebron11:02:42

Paren Mode only knows about indentation thresholds, and will simply clamp existing indentation to these thresholds.

shaunlebron11:02:05

You are right that correcting indentation is semantic and based on several things, including user-preference

shaunlebron11:02:18

You can think of Paren Mode as the minimal set of operations required to make code editable under Indent Mode

shaunlebron11:02:51

(i.e. where does the indentation need to be such that Indent Mode won’t relocate any parens?)

shaunlebron11:02:55

the cursorDx option was meant to preserve your configured indentation (1-space, 2-space, n-space) as you type

shaunlebron11:02:29

so that it falls on the user to choose the correct indentation within a threshold, and then Paren Mode would try to respect what you chose

shaunlebron12:02:23

> Indent Mode does not correct indentation

shaunlebron12:02:42

regarding this point, Indent Mode only correct parens, and will never correct your indentation for you

shaunlebron12:02:20

In your example, the cursor is holding the recently-typed paren and preventing its relocation as a necessary courtesy

shaunlebron12:02:39

you’ll notice that the last link goes to Part 2 of the Parinfer design doc. The two parts are: 1) the simpler parts for processing static documents, and 2) the extra UX parts needed for a live-edited document

shaunlebron12:02:49

The undo problem is related to keeping Parinfer change events off the undo history stack

shaunlebron12:02:48

We couldn’t do this in Sublime, so Chris labeled Parinfer’s change events, and then rebound Ctrl+Z to run undo twice if the top of the undo stack was created by Parinfer

shaunlebron12:02:25

The caret repositioning problem is difficult, because it requires the algorithm to know the exact change(s) that occurred

shaunlebron12:02:18

I have three cases listed there, but let me know if you find any more

shaunlebron12:02:22

Ha, Paren Mode is less about mathematical symmetry for its own sake, and more about creating a complete system to explore what is useful to the users

shaunlebron12:02:51

since it’s early on, there isn’t much data on that

shaunlebron12:02:41

but some are reporting cases where they expected indentation to be corrected in Indent Mode

shaunlebron12:02:54

so Paren Mode exists for these small cases where we haven’t yet figured out how else to do this without requiring a separate mode

shaunlebron12:02:39

see avoid fracturing and indentation is maintained examples here: http://shaunlebron.github.io/parinfer/#paren-mode

shaunlebron12:02:46

but we’ve started thinking about fusing the modes here: https://github.com/shaunlebron/parinfer/issues/86

shaunlebron12:02:27

@cfleming: ^^ thanks for all feedback and questions. sorry about the caret movement problem. i think your incremental changes solution will help since I’m not planning a near-term fix that would require the modes to know exact changes made

shaunlebron13:02:25

Actually @cfleming, I’ll work on the caret movement this week, I think I can solve it in the API now without change information (blunder): https://github.com/shaunlebron/parinfer/issues/47#issuecomment-184212306

shaunlebron14:02:54

about your last point, I’ll think about whether pressing enter should displace the paren or not. it makes sense not to from the algorithm’s perspective, but not the user perspective

chrisoakman17:02:24

great questions and answers here simple_smile

chrisoakman17:02:15

for undo, if IntelliJ allows you to apply an edit and skip the undo stack, then that's the cleanest option

chrisoakman17:02:05

rather, I should say that the Atom API offers that, and atom-parinfer takes advantage of it

chrisoakman17:02:38

else you have come up with a similar solution to what we did in the sublime plugin

snoe20:02:20

The undo problem is why I changed from async to sync in nvim-parinfer, in vim undojoin basically rolls a result into the previous undo entry. Depending on the editor it may be able to do something similar even asynchronously. https://github.com/snoe/nvim-parinfer.js/blob/master/plugin/parinfer.vim#L22-L25

cfleming23:02:40

I got an answer in the JetBrains forum about the undo problem, so I think I can fix that. They’re stepping up their game on the support.

cfleming23:02:51

There’s another interesting undo case. When a new file is opened in indent mode, paren mode is run to prep the file. However this also needs to happen when the user turns indent mode on in an existing open file. I’m showing a notification of the changes made by paren mode (“Indentation fixed for <x> lines”), and if the user realises that that is not what they want and they undo, that undo should also turn indent mode off again. It’s quite tricky but I think I can do it.

cfleming23:02:38

@shaunlebron: Yes, I think the caret movement should be relatively simple to implement. In your issue you’re mapping the cases to logical operations, but I think it will be easier if you think of the changes that are applied as modifications to the document. Basically, if the line the cursor is currently on is modified, then a text deletion of any kind before the cursor means that its offset needs to be decremented by the width of that text, and and insertion of any kind before the cursor means that it needs to be incremented by the width. The only tricky case is when an insertion happens at the caret - you’ll need to work out whether it should go before or after it.

cfleming23:02:45

The more I think about it, the more I’m tempted to only implement indent mode. I think the fact that indentation is semantic means that paren mode as it stands won’t be useful as a general editing mode, and actually using the correct indentation will be editor specific and quite possibly too slow to be interactive. For the initial indentation correction on file open, I think I’ll modify the paren mode algorithm to just record the lines that need correction, and then indent them (slowly but accurately) with the existing formatting code.

cfleming23:02:47

If people complain and it turns out that it really is necessary, I’ll either add it then or try a hybrid as suggested in #86

cfleming23:02:50

> In your example, the cursor is holding the recently-typed paren and preventing its relocation as a necessary courtesy I don’t think this is correct, since the cursor has been (incorrectly) moved to the previous line anyway. Indent mode only fixes that paren when another modification is made to the doc. This also seems wrong, since it’s relatively trivial for the user to get the doc into a state which violates the parinfer invariants. I think the indentation should be modified in that case - I guess I’m really asking for the hybrid mode.

cfleming23:02:07

Thinking about it, the algorithm thinks that it’s holding the recently-typed paren, but this is hidden by the fact that the cursor has moved. However, this seems incorrect, that implies that indent mode should be run again on a caret movement where the caret leaves the line with the held paren. However that means that you type a paren, it’s held but then it’s immediately moved back when you leave the line. I think the conclusion of this is that in indent mode, the user should not actually be allowed to type close parens.