Clojurians Log v2

my feeling is that this cursorDx solution is a partial one, but still useful enough for an early version, since it is not introducing more problems to Indent Mode, but reducing them

Here’s a test case:

((if some-condition
   print
   println) {:foo 1
             :bar 2})

and the smart mode can be a complete solution that takes an array of changes, but will require some work

If you wrap the whole (if some-condition ... ), then the print and println should be indented by 1, and the map by 2.

nice case

It gets really hard with overlapping changes, too.

do you control the wrap operation?

could it replaced with a single change like atom does?

(not paredit I mean)

Well, I could do, but that then goes down the path of special-casing editing ops.

It really needs a general solution to be robust.

And I can’t special-case my way out of renaming, either.

makes sense

like:

(my-fn (if some-condition
         print
         println) my-fn {:foo 1
                         :bar 2})

If I rename my-fn there, for example.

(very contrived, of course, but I’m sure there are plenty of real cases).

I think the changes option is doable for handling renames like this

I track both input and output coordinates as I parse the text

i’ll have to think more about it, but it feels doable

Here’s how I think this might work:

Really, the cursorDx at the moment is “from some point in the file (the new end offset of a change), indents must be changed by dx until a close paren”

Is that correct?

yes

So it seems like you could accept a list of those in result-document-order, and essentially keep a stack of them - they’re scoped by the parens, really.

When you encounter a new one, it gets pushed on the stack, and when you reach the corresponding close paren, you pop it.

The total dx at any point in time is the sum of the dxes on the stack.

At least in IntelliJ, calculating that list is relatively easy.

I think that’s right, and it actually should be relatively simple to implement.

It works nicely with the way parinfer works right now.

“result-document-order”?

Really, a cursorDx is the dx value and the offset in the doc after which to apply it.

So just ordered by the offsets in the document.

Whoever gives them to you will have to take care that they’re in the correct order, and also that they’re updated to use the co-ordinates in the resulting document.

sounds good

what would you call this option? it should replace cursorDx

changeDx

Or editDx, perhaps.

It’s the change in the indentation caused by a particular change or edit.

But perhaps just call it changes? It will be a list of [offset dx] pairs.

maybe I should just take the changes and compute the [offset dx] internally

I suspect there are hairy edge cases here if the changes are not balanced, thinking about it.

and then we can use that option for other things later

Imagine that a user selects some text including an open paren, and then replaces it with text containing no parens.

what problem would that cause?

Because these will be applied using the parens for scoping. Let me see if I can think of a case.

(test {:foo 1
       :bar 2})

Imagine that the user selects test { and replaces it with foo.

I guess that would work, actually, since the closing } will be removed during processing anyway

I wonder if there are tricky cases when the user deletes closing parens.

Anyway, I guess we’ll find them soon enough.

yeah, I couldn’t think of a problem with it actually, other than causing extra character insertion that could throw off the other change coordinates if I’m doing it incorrectly

will look at this after dinner, thanks so much guys. couldn’t have done this myself

Thanks to you too - I’m really excited for this change!

Actually….

Even more exciting, I think this can be used for the partial update change.

As you’re processing, you record more or less the same data you’re doing now, but don’t make any modifications.

When you encounter a change, you know when the previous open paren was. You then process, making modifications, until that paren is balanced again.

OMG

Then continue without making modifications until you reach another change.

In fact, it’s possible you could have two modes - scanning and only checking that parens are balanced, and then a mode where you’re processing, basically exactly as you do now. You just need to set up the correct data at the start of the open paren previous to the change.

ha, not following yet

I’m looking after our daughter now, I’ll try to explain better later.

But I think this might be the change which achieves everything I wanted from parinfer 🙂

streaming some work tonight: https://www.twitch.tv/shaunlebron

@shaunlebron So I basically wanted to change two things from parinfer, which I was planning to investigate. One was removing the indent/paren mode difference. The other was only processing the parts of the file (sexps, probably) affected by a particular change. This would mean that people wouldn’t have to pre-format their files on file open, and it would not be a requirement that the whole file complied with the indent rules - just the parts that they’re editing. My idea was to detect while processing the affected part of the file if that particular section didn’t comply with the indent rules, and to show an error at that point. So if the user tried an action in a part of the file which couldn’t support indent mode, parinfer just wouldn’t get run, they would be notified and they would have to fix it by hand, reformat it or something similar.

This change you’re making fixes the first, and I believe it can also be made to do the second.

I think the best way to do this would be to start scanning the file in a non-modifying mode. This would scan the file contents much like parinfer does at the moment, but it would not actually modify the file, it would just record the paren stack, the current horizontal position and the position of the last open paren (that might be implicitly stored in the paren stack).

Then, when you encounter one of these changes, you switch to processing exactly like parinfer does now, or at least like your new modified version does. You record the last open paren you saw. You update the file, but critically you stop processing as soon as the last open paren you recorded earlier is balanced. This means that the paren trail handling is a little trickier since you don’t just delete all close parens wholesale, you only do it until they’re balanced. Once your original open paren is balanced again, you switch back to your non-modifying scanning mode again until you hit another change.

If you ever get to a stage where the parens can’t balance, you barf with an error - this is like your new v2 warning mode. Similarly, you barf with an error if the sections you’re actually processing don’t obey the indent mode rules - I’m not sure if this requires more information to be tracked than is currently, but I suspect it’s a relatively simple change.

Obviously I haven’t implemented this, but in my head it works perfectly 🙂

The only bit that I don’t have quite clear in my head (been too long since I looked at all the details) is that the close paren tracking & matching will work correctly - I’d need to try it.

Actually, thinking about this, I’m not sure it will work - I don’t know if parinfer can work out when processing should stop, since normally it relies on removing all close parens and putting them back where it infers them.

I think it will probably need the original range of the sexp from the original document, which gets back to the complexity around tracking ranges across all the changes.

just thinking through an example:

(defn foo
  ([a]
     (foo a 123)) ;; <--- what happens when adjusting indentation of this line?
  ([a b]
     (+ a b)))

having a hard time piecing together expected behavior here, maybe we can work from this example to see what would be partially processed

some questions: 1. should dedenting this line by one space cause it to enter the parent form? 2. should dedenting all the way cause it to adopt everything below it?

@rgdelato: I published 2.5.2

you can either run with that, or wait for a cursorDx to be replaced with a changes array option

i’m leaving for a road trip tomorrow, and I may not finish the changes thing today

cool, have fun on your trip!

@shaunlebron > maybe I should just take the changes and compute the [offset dx] internally I think this would be a good idea BTW, then you have more context if you want it later. It means explaining the concept of the different co-ordinate spaces and specifying which you want though.

More thoughts later

thanks colin!

streaming here for the changes option: http://twitch.tv/shaunlebron

@shaunlebron So that’s a tricky case to start with 🙂

One thing I’m not clear on is if when you encounter a change, if you need to reprocess from the last opening paren. I’m not sure how much data you would need to collect to begin processing immediately when encountering a change.

So I think assuming that either you reprocess from the opening paren, or you do have enough data from the initial scanning, when adjusting the indentation of that line the last opening paren will be the ([a]...

i.e. we’ll try to process the whole arity 1 body.

But as soon as we start processing it, we’ll find that it’s not indented according to the indent mode rules, and fail with an error stating that.

Let’s start with an example that actually works first.

(let [{:keys [refer rename]} filters
      name (get rename place-name place-name)]
  (when (or (= :all refer) ; <-- replace this when with an if
            (contains? refer name))
    name))

After replacing:

(let [{:keys [refer rename]} filters
      name (get rename place-name place-name)]
  (if (or (= :all refer) ; <-- change is right after if, dx = -2
            (contains? refer name))
    name))

Now, when you run parinfer, you’ll scan the file, not modifying anything but just checking parens are balanced, until you hit the change.

So your last open paren is right before the if, and you start processing. I’m assuming that at that point you’re processing in paren mode, because you have an active dx.

I haven’t seen that code, so I don’t know exactly how that works, but I’m assuming that dx is in scope for the following block, i.e. the (or ...) sexp, so that all gets indented correctly:

(let [{:keys [refer rename]} filters
      name (get rename place-name place-name)]
  (if (or (= :all refer)
          (contains? refer name)) ; <- this gets indented by dx
    name))

then you continue processing until the end of name). At that point you realise that your original open paren (if... is now balanced, so you go back to scanning. Since there are no more changes, you’re done.

So you only made edits inside the if sexp, and the forms in the let binding vector could have been incorrectly indented (according to indent mode rules), and that wouldn’t have been a problem as long as the parens are correctly balanced.

If something inside the if block were incorrectly indented, you’d fail with an error.

Here’s another case:

(let [{:keys [refer rename]} filters
      name (get rename place-name place-name)]
  (if (or (= :all refer) ; <- caret is now right before (or…, and I type or paste (and 
          (contains? refer name))
    name))

Because I’m going to make my conditional more complicated.

So now we have:

(let [{:keys [refer rename]} filters
      name (get rename place-name place-name)]
  (if (and (or (= :all refer) ; <- and is currently unbalanced, following code is not indented
          (contains? refer name))
    name))

Again, assuming that dx is in scope for the or block, you process that in paren mode:

(let [{:keys [refer rename]} filters
      name (get rename place-name place-name)]
  (if (and (or (= :all refer) ; <- and is still unbalanced
               (contains? refer name))
    name))

Then at the end of the (or...) block, you’re processing in indent mode again as normal, and add the closing paren for the (and...

(let [{:keys [refer rename]} filters
      name (get rename place-name place-name)]
  (if (and (or (= :all refer)
               (contains? refer name)))
    name))

Then as before, you keep processing until the end of name), switch to scanning and run to the end.

The tricky part here (and in the previous case) is how to handle the paren trail for name))

Actually, looking back at the existing paren trail rules, I’m not sure they need to be any different.

The one tricky bit might be: IIRC at the end of a file, if there are any remaining outstanding close parens they’re inserted at the end to ensure everything is balanced.

Should that happen here when the open paren before the change is balanced? I guess by definition there can be no outstanding close parens at that point.

2.6.0 pushed with changes option that replaces cursorDx

@rgdelato @chrisoakman we can start integrating the changes option into atom-parinfer now

documented here: https://github.com/shaunlebron/parinfer/tree/master/lib#api

I’ll try to update the site demo tonight if I have time 🙂

don’t sweat it.

Assuming this looks good in Atom I’ll port to Kotlin and try it in IntelliJ.

And I’ll experiment with the partial change thing too.

Thanks for this!