parinfer / parinfer.js

Let's simplify the way we write Lisp
https://shaunlebron.github.io/parinfer
MIT License
1.76k stars 40 forks source link

Bug in multi-line Raise operation #185

Closed cursive-ide closed 6 years ago

cursive-ide commented 6 years ago

I have the following code. It's oddly (but legally) indented because it's an intermediate step in a refactor:

(then (-> (dynamo/get-item @db {})
          (then #(is (= % {:quote-id quote-id :more-data "Some data"}))
            (fn [err result]
              (if err
                (is false (str "Unexpected error: " (error err)))
                (is (= result {:quote-id quote-id :more-data "Some data"})))
              (done)))))

I then use paredit's raise operation to raise the (-> ...), deleting the surrounding (then ... ). I end up with:

(-> (dynamo/get-item @db {})
    (then #(is (= % {:quote-id quote-id :more-data "Some data"}))
    (fn [err result]
    (if err
    (is false (str "Unexpected error: " (error err))
              (is (= result {:quote-id quote-id :more-data "Some data"}))
            (done))))))

Note that the raise operation is a combination of two deletes from the original code: (then and ), and then a bunch of whitespace manipulation to indent the resulting form. Here is the code to reproduce this in the sandbox:

const code = `
(-> (dynamo/get-item @db {})
    (then #(is (= % {:quote-id quote-id :more-data "Some data"}))
          (fn [err result]
            (if err
              (is false (str "Unexpected error: " (error err)))
              (is (= result {:quote-id quote-id :more-data "Some data"})))
            (done))))
`;

console.log(parinfer.smartMode(code, {
  changes: [
    {lineNo: 7, x: 23, oldText: ')', newText: ''},
    {lineNo: 1, x: 0, oldText: '(then ', newText: ''},
    {lineNo: 2, x: 4, oldText: '      ', newText: ''},
    {lineNo: 3, x: 10, oldText: '  ', newText: ''},
    {lineNo: 4, x: 12, oldText: '  ', newText: ''},
    {lineNo: 5, x: 14, oldText: '  ', newText: ''},
    {lineNo: 6, x: 14, oldText: '  ', newText: ''},
    {lineNo: 7, x: 12, oldText: '  ', newText: ''},
  ],
}).text);

The set of edits looks reasonable to me, but the end result is clearly incorrect.

sooheon commented 6 years ago

Is this in Cursive? I was not able to reproduce with copy paste. Latest EAP of cursive, Parinfer enabled.

cursive-ide commented 6 years ago

@sooheon Yes, it is. You'll have to be careful that IntelliJ doesn't reformat on copy/paste. Turn parinfer off (structural editing: none), then Editor->General->Smart Keys->Reformat on paste->None. Then paste the code from here, re-enable parinfer and reproduce.

shaunlebron commented 6 years ago

Is there a way for Cursive to disable Parinfer, before applying this change, then reapply after?

I don't think it's feasible to continue accreting complexity to Parinfer for large edit chains like this.

cursive-ide commented 6 years ago

That is actually a really great idea that I can't believe I hadn't thought of. Yes, I should be able to tag certain actions as not requiring parinfer to be run after. Any actions which I'm confident won't mess up the structure (i.e. the paredit ones, and things like intention actions) would work like this.