Change tracking is broken

[justinbarclay]

There is currently a bug in change tracking that causes us to send the wrong information to parinfer-rust. This causes smart-mode to make wrong corrections in turn.

This bug doesn't manifest itself in "regular" use but can break when used with undo or in conjunction with paredit-slurp/barf.

[andreyorst]

maybe it's not related, but it would be nice if smart mode preserved structure when we join lines with delete-indentation bound by default to M-^ (Shift+Alt+6). Currently it acts as if we were in indent mode: In:

Current smart mode delete-indentation outcome:

Expected outcome for smart mode:

This is what happens in Atom's parinfer package, and when using parinfer-rust in Kakoune editor

[justinbarclay]

As a follow up to my initial report and @andreyorst example, I've been exploring an augmented change system in Emacs. This system tracks the changes in the buffer and then combines changes into an area of change based on some simple metrics before passing the changes into parinfer.

The reason that I am putting a lot of effort into detecting and reporting changes is because they are very important for parinfer smart-mode. Smart mode uses the current text and the set of changes applied to the text to determine how to manage your parenthesis.

But this leads to a problem, the change information that Emacs gives to us is quite minimal. Generally, it's a list (region-start region-end length). This is fine when the change are triggered directly by the user, inserting or deleting characters, by fine I mean the change reported is necessary and sufficient to get parinfer to make the right decision.

However, I am finding the changes being reported by Emacs to be insufficient to report to parinfer the proper information to have it make the correct decisions.

For example, I have a buffer much like the one above:

(def thing
  {:abc 1
   :xyz 2})

Placing my cursor just before the { and calling delete-indentation causes very similar set of changes to be shown as those reported by @andreyorst

(def thing {:abc 1}
   :xyz 2)

Digging into this further, my system reports these sets of changes (I've added some more information to be used in my change consolidation system).

 ((lineNo 0 x 10 start 11 end 11 length 1 group-p nil)
(lineNo 0 x 10 start 11 end 11 length 2 group-p nil)
(lineNo 0 x 10 start 11 end 12 length 0 group-p nil))

I find this hard to visualize, so converting that to just text it looks like...

("", "", " ")

We also know that two of these changes are deletions because length is 1 and 2 respectively so those two deletions are just whitespace. For visualization the before regions of text look like:

(" ", "  ", " ")

No new lines detected.

So this means there are either bugs in my system or Emacs doesn't report thorough enough changes. I fully admit that my system might be buggy, or that I might be making the wrong assumptions to how this system works. I am still pretty new to writing Emacs packages. But it is apparent to me that this is going to take much more time to resolve then I had previously thought.

In the interlude, I am thinking of adding in a new system for users to specify what mode specific commands should run under.

IE A user could say that delete-indentation mode should run under "paren" forcing alignment to be consistent. (And honestly I think most non individual text insertion/delete char commands should be run with the assumption that parinfer will maintain parens)

[justinbarclay]

After my last failure of doing parinfer compatible change tracking, I've hit a roadblock on how to continue. I still plan on working on this but it's going to take a lot of hammock time. In the interim I have added an escape hatch, parinfer-rust-treat-command-as.

From the docs:

parinfer-rust-treat-command-as is an escape hatch for smart mode that allows you to tell parinfer-rust-mode what mode to run a specific command. parinfer-rust-treat-command-as is a list of pairs.The first item in the pair specifies the command and the second item in the pair specifies the mode the command should be run under. For example '(delete-indentation . "paren"), tells parinfer-rust-mode to override smart mode and run under paren mode when it detects that delete-indentation caused a change in the buffer.

You can extend to parinfer-rust-treat-command-as using ~add-to-list~ as shown below:

 (add-to-list 'parinfer-rust-treat-command-as '(delete-indentation . "paren")) 
 ;;or
 (add-to-list 'parinfer-rust-treat-command-as '(delete-indentation. "indent"))

I realize it's inelegant but it should help prevent smart mode from unexpectedly breaking code.

[andreyorst]

I wonder if all non-user-input events should happen in paren mode... This seem to sound like a proper way, since even things like formatting should not move parens

[andreyorst]

(add-to-list 'parinfer-rust-treat-command-as '(delete-indentation . "paren"))

Doesn't seem to work

[justinbarclay]

I should not push out code late at night 😅.

I pushed out a fix for treat-command-as and it should be comparing against the right command this time.

[justinbarclay]

Yeah, I've had the thought of tracking the use of self-insert-action and some subset of delete/kill actions to treat them explicitly as smart mode and then using paren mode for the rest of the commands. But there seems to be alot of edge cases to handle around delete actions making the right choice of being smart or paren based. So, I've just chosen to see how good I can make change tracking for now.

[andreyorst]

(add-to-list 'parinfer-rust-treat-command-as '(delete-indentation . "paren"))

Doesn't seem to work

This now seem to work great

[justinbarclay]

I've done some work around change tracking that includes merging changes that occur sequentially in time and on the same start position.

This has really helped with smart mode handling delete-indentation properly and you might not need to add it to that list anymore.

Either way I am closing this issue as I think change tracking is in a lot better state since I filed this.