Skip to content

Commit

Permalink
Update to the track changes library (#84)
Browse files Browse the repository at this point in the history
* Switch to track-changes library

Initial application of Stefan Monnier's patch

* Try unwind-protect

* Chore: adjust whitespace

* Switch to using change-signal to trigger parinfer-rust--execute

* Fix bugs in change tracking implementation

Remove call at beginning of execute

There should only be one entry point to change tracking and that entry
point triggers execute

Unregister change tracker

Make things local

* Switch to more refined change hook
  • Loading branch information
justinbarclay authored May 2, 2024
1 parent ef99d42 commit a88eff7
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 140 deletions.
132 changes: 32 additions & 100 deletions parinfer-rust-changes.el
Original file line number Diff line number Diff line change
Expand Up @@ -65,84 +65,12 @@
(require 'parinfer-rust parinfer-rust-library t)

(require 'parinfer-rust-helper)
(require 'track-changes)
(require 'subr-x)

(defvar-local parinfer-rust--changes '()
"The current set of unprocessed changes.")

(defun parinfer-rust--merge-changes (change-a change-b)
"Return change list from CHANGE-A and CHANGE-B.
Return the set of changes that covers the greatest region, the
lowest start value, highest end value, and merge the before and
after text for two changes."
(let ((start (if (< (plist-get change-a 'start)
(plist-get change-b 'start))
(plist-get change-a 'start)
(plist-get change-b 'start)))
(end (if (> (plist-get change-a 'end)
(plist-get change-b 'end))
(plist-get change-a 'end)
(plist-get change-b 'end)))
(length (+ (plist-get change-a 'length)
(plist-get change-b 'length))))
(list
'lineNo (plist-get change-a 'lineNo)
'x (plist-get change-a 'x)
'start start
'end end
'length length
'before-text (string-join (list (plist-get change-a 'before-text)
(plist-get change-b 'before-text)))
'after-text (string-join (list (plist-get change-a 'after-text)
(plist-get change-b 'after-text)))
'group t)))

(defun parinfer-rust--combine-changes (change-list)
"Iterate over CHANGE-LIST and look for change.
Changes that operate beside each other sequentially in time and
on similar regions of texts."
(let ((sorted-changes (reverse change-list))
(consolidated-changes '())
(previous-line nil)
(previous-start nil))
(dolist (change sorted-changes consolidated-changes)
;; Look for text
(if (and (equal previous-line
(plist-get change 'lineNo))
(equal previous-start
(plist-get change 'start)))
(setq consolidated-changes
(cons
(parinfer-rust--merge-changes (car consolidated-changes) change)
(cdr consolidated-changes)))
(setq consolidated-changes (cons change consolidated-changes)))
(setq previous-start (plist-get change 'start))
(setq previous-line (plist-get change 'lineNo)))))

;; Good for future tests
;; (setq some-changes
;; '((lineNo 7 x 10 start 170 end 171 length 0 before-text "" after-text " " group nil)
;; (lineNo 7 x 10 start 170 end 170 length 2 before-text " " after-text "" group nil)
;; (lineNo 7 x 10 start 170 end 170 length 1 before-text "\n" after-text "" group nil)))

;; (assert
;; (equal
;; '(lineNo 7 x 10 start 170 end 170 before-text "\n " after-text "" length 3 group t)
;; (parinfer-rust--merge-changes
;; '(lineNo 7 x 10 start 170 end 170 length 1 before-text "\n" after-text "" group nil)
;; '(lineNo 7 x 10 start 170 end 170 length 2 before-text " " after-text "" group nil))))

;; (assert
;; (equal
;; (parinfer-rust--combine-changes some-changes)
;; '((lineNo 7 x 10 start 170 end 171 before-text "\n " after-text " " length 3 group t))))
(defun parinfer-rust--get-before-text (start end)
"Text before change using START and END."
(setq-local parinfer-rust--before-text-change
(buffer-substring-no-properties start end)))

(defun parinfer-rust--build-changes (change-list)
"Convert CHANGE-LIST to a list of change structs for parinfer-rust."
(let ((changes (parinfer-rust-make-changes)))
Expand All @@ -157,34 +85,38 @@ on similar regions of texts."
(setq-local parinfer-rust--changes '())
changes))

(defun parinfer-rust--track-changes (start end length)
"Track change in buffer using START, END, and LENGTH.
Uses START, END, and Length to capture the state from the
previous buffer and current buffer."
(if parinfer-rust--disable
(defun parinfer-rust--fetch-changes (id)
"Fetch changes for current buffer using signal ID."
(track-changes-fetch id
(lambda (start end before)
(if parinfer-rust--disable
nil
;; If we're in test-mode we want the absolute position otherwise
;; relative is fine
(let ((lineNo (- (line-number-at-pos start t) 1))
(x (save-excursion
(save-restriction
(widen)
(goto-char start)
(parinfer-rust--get-cursor-x)))))
(push (list 'lineNo lineNo
'x x
'start start
'end end
'length (length before)
'before-text before
'after-text (buffer-substring-no-properties start end)
'group nil)
parinfer-rust--changes))))))

(defun parinfer-rust--changes-signal (id &optional distance)
"Signal changes for ID with optional DISTANCE."
(parinfer-rust--fetch-changes id)
(if distance
;; We're still in the middle of changes, but they're "far",
;; so record the past changes and keep going.
nil
;; If we're in test-mode we want the absolute position otherwise relative is fine
(let* ((lineNo (- (line-number-at-pos start t)
1))
(x (save-excursion
(save-restriction
(widen)
(goto-char start)
(parinfer-rust--get-cursor-x)))))
(push (list 'lineNo lineNo
'x x
'start start
'end end
'length length
'before-text (or parinfer-rust--before-text-change "")
'after-text (buffer-substring-no-properties start end)
'group nil)
parinfer-rust--changes))
(setq parinfer-rust--previous-buffer-text
(save-restriction
(widen)
(buffer-substring-no-properties (point-min) (point-max))))))
(parinfer-rust--execute)))

;; Local Variables:
;; package-lint-main-file: "parinfer-rust-mode.el"
Expand Down
89 changes: 49 additions & 40 deletions parinfer-rust-mode.el
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
;; Author: Justin Barclay <[email protected]>
;; URL: https://github.com/justinbarclay/parinfer-rust-mode
;; Version: 0.8.6
;; Package-Requires: ((emacs "26.1"))
;; Package-Requires: ((emacs "26.1") (track-changes "1.1"))
;; Keywords: lisp tools

;; This program is free software: you can redistribute it and/or modify
Expand Down Expand Up @@ -125,8 +125,8 @@
;;; Code:

;; For a complete list of state that needs to be tracked read:
;; https://github.com/shaunlebron/parinfer/tree/master/lib#api
;; https://github.com/shaunlebron/parinfer/blob/master/lib/doc/integrating.md
;; https://github.com/parinfer/parinfer.js/tree/master#api
;; https://github.com/parinfer/parinfer.js/blob/master/doc/integrating.md

;; In broad strokes we must:

Expand All @@ -145,6 +145,7 @@ against and is known to be api compatible.")

;; Require helper so we can check for library
(require 'parinfer-rust-helper)
(require 'track-changes)

(eval-when-compile
(declare-function parinfer-rust-make-option "ext:parinfer-rust" t t)
Expand Down Expand Up @@ -295,7 +296,9 @@ Either `paren', `indent', or `smart'.")
(defvar-local parinfer-rust--ignore-post-command-hook nil
"A hack to not run parinfer-execute after an undo has finished processing.")

(defvar parinfer-rust--last-mode nil
(defvar-local parinfer-rust--change-tracker nil)

(defvar-local parinfer-rust--last-mode nil
"Last active Parinfer mode.
Used for toggling between paren mode and last active mode.")
;;;;;;;;;;;;;;;;;;;;;;;;;
Expand All @@ -317,24 +320,25 @@ parinfer."
(setq-local parinfer-rust--disable nil))


;; The idea for this function: 1. is to never run during an undo operation 2. Set a flag to ignore
;; the first post command execution after an undo operation 2 is important because if we undo our
;; last key press and that causes parinfer to modify the buffer we get stuck in a loop of trying to
;; undo things and parinfer redoing them

;; The idea for this function:
;;
;; 1. is to never run during an undo operation
;;
;; 2. Set a flag to ignore the first post command execution after an undo operation
;;
;; 2 is important because if we undo our last key press and that causes parinfer to modify the
;; buffer we get stuck in a loop of trying to undo things and parinfer redoing them
(defun parinfer-rust--track-undo (orig-func &rest args)
"Wraps ORIG-FUNC and ARGS in some state tracking for `parinfer-rust-mode'."
(condition-case-unless-debug err
(apply orig-func args)
(unwind-protect
(apply orig-func args)
;; We want to "Ignore" errors here otherwise the function exits
;; and causes the following commands, where we set flags, to be
;; ignored
(error
(message "%s" (cadr err))
nil))
;; Always ignore the first post-command-hook run of parinfer after an undo
(setq-local parinfer-rust--ignore-post-command-hook t)
(parinfer-rust--set-default-state))
(progn
;; Always ignore the first post-command-hook run of parinfer after an undo
(setq-local parinfer-rust--ignore-post-command-hook t)
(parinfer-rust--set-default-state))))

(defun parinfer-rust--execute-change-buffer-p (mode)
"Return non-nil if running `parinfer-rust--execute' with MODE would change the current buffer."
Expand Down Expand Up @@ -401,9 +405,7 @@ CHANGES."
(old-options (or (parinfer-rust--local-bound-and-true parinfer-rust--previous-options)
(parinfer-rust-make-option)))
(changes (if (> (length parinfer-rust--changes) 0)
(parinfer-rust--build-changes
(parinfer-rust--combine-changes
parinfer-rust--changes))
(parinfer-rust--build-changes parinfer-rust--changes)
(parinfer-rust-make-changes)))
(options (parinfer-rust--generate-options old-options
changes))
Expand All @@ -414,9 +416,6 @@ CHANGES."
(answer (parinfer-rust-execute request))
(replacement-string (parinfer-rust-get-in-answer answer "text"))
(error-p (parinfer-rust-get-in-answer answer "error")))
;; We don't want other hooks to run while we're modifying the buffer
;; that could lead to weird and unwanted behavior
(setq-local inhibit-modification-hooks t)
(when (and (local-variable-if-set-p 'parinfer-rust--in-debug)
parinfer-rust--in-debug)
(parinfer-rust-debug "./parinfer-rust-debug.txt" options answer))
Expand Down Expand Up @@ -444,8 +443,9 @@ CHANGES."
(new-line (parinfer-rust-get-in-answer answer "cursor_line")))
(parinfer-rust--reposition-cursor new-x new-line))
(setq parinfer-rust--previous-options options)
(with-no-warnings ;; TODO: Should not need with-no-warnings function
(setq-local inhibit-modification-hooks nil))))))
(when parinfer-rust--change-tracker
;; Ignore our own changes.
(track-changes-fetch parinfer-rust--change-tracker #'ignore))))))

;; Interactive or functions meant to be called by user
(defun parinfer-rust-toggle-debug ()
Expand Down Expand Up @@ -484,10 +484,17 @@ Disable `parinfer-rust-mode' if the user does not want to have
parinfer autofix them, or if there is no reasonable way for
`parinfer-rust-mode' to automatically fix them."
(setq-local parinfer-rust--disable nil)
;; Disable change tracker for now because we are about to make changes in an change hook.
(track-changes-unregister parinfer-rust--change-tracker)
(setq-local parinfer-rust--change-tracker nil)
(if-let (issue (or (parinfer-rust--check-for-tabs)
(parinfer-rust--check-for-indentation)))
(parinfer-rust-mode -1))
(remove-hook 'before-change-functions #'parinfer-rust--check-for-issues t))
(parinfer-rust-mode -1)
;; Re-enable change trackers now that we've succeeded in our tasks
(setq-local parinfer-rust--change-tracker
(track-changes-register #'parinfer-rust--changes-signal
:disjoint t)))
(remove-hook 'first-change-hook #'parinfer-rust--check-for-issues t))

(defun parinfer-rust--switch-mode (&optional mode)
"Switch to a different Parinfer MODE.
Expand All @@ -511,24 +518,27 @@ Checks if MODE is a valid Parinfer mode, and uses
(parinfer-rust--set-default-state)
(setq-local parinfer-rust--mode parinfer-rust-preferred-mode)
(advice-add 'undo :around #'parinfer-rust--track-undo)
(when (fboundp 'undo-tree-undo)
(advice-add 'undo-tree-undo :around #'parinfer-rust--track-undo))
;; Track changes as they happen in the buffer
(add-hook 'before-change-functions #'parinfer-rust--get-before-text t t)
(add-hook 'after-change-functions #'parinfer-rust--track-changes t t)
;; Run parinfer after whatever command caused the change(s) has finished
(add-hook 'post-command-hook #'parinfer-rust--execute t t)
(advice-add 'undo-tree-undo :around #'parinfer-rust--track-undo)
(if (fboundp 'track-changes-register)
(progn
(when parinfer-rust--change-tracker
(track-changes-unregister parinfer-rust--change-tracker)
(setq-local parinfer-rust--change-tracker nil))
(setq-local parinfer-rust--change-tracker
(track-changes-register #'parinfer-rust--changes-signal
:disjoint t))))
(parinfer-rust--dim-parens))

(defun parinfer-rust-mode-disable ()
"Disable Parinfer."
(advice-remove 'undo #'parinfer-rust--track-undo)
(when (fboundp 'undo-tree-undo)
(advice-remove 'undo-tree-undo #'parinfer-rust--track-undo))
(remove-hook 'before-change-functions #'parinfer-rust--get-before-text t)
(remove-hook 'after-change-functions #'parinfer-rust--track-changes t)
(remove-hook 'post-command-hook #'parinfer-rust--execute t)
(when parinfer-rust--change-tracker
(track-changes-unregister parinfer-rust--change-tracker)
(setq-local parinfer-rust--change-tracker nil))
(setq-local parinfer-rust-enabled nil)
(remove-hook 'first-change-hook #'parinfer-rust--check-for-issues t)
(parinfer-rust--dim-parens))

(defun parinfer-rust-toggle-disable ()
Expand Down Expand Up @@ -561,12 +571,11 @@ This includes stopping tracking of all changes."
buffer-read-only)
;; Defer checking for changes until a user changes the buffer
(setq-local parinfer-rust--disable t)
(add-hook 'before-change-functions #'parinfer-rust--check-for-issues t t))
(add-hook 'first-change-hook #'parinfer-rust--check-for-issues nil t))

((eq 'immediate parinfer-rust-check-before-enable)
(setq-local parinfer-rust--disable t)
(parinfer-rust--check-for-issues))

(t (let ((parinfer-rust--mode "paren"))
(parinfer-rust--execute)))))
(progn
Expand Down Expand Up @@ -608,7 +617,7 @@ not available."
:init-value nil
:keymap parinfer-rust-mode-map
(cond
(parinfer-rust-enabled
(parinfer-rust-enabled ;; FIXME: Why?
(parinfer-rust-mode-disable))
;; Don't do anything if the buffer is not selected
;; TODO: Come up with a better way to defer and disable loading
Expand Down

0 comments on commit a88eff7

Please sign in to comment.