From: João Távora Date: Wed, 22 Jan 2025 15:44:41 +0000 (+0000) Subject: Eglot: abandon track-changes.el X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=4b34c81d62c4c44cae57f88489a11b3e3cf60386;p=emacs.git Eglot: abandon track-changes.el After a ~10 month period of using track-changes.el as a support library for tracking buffer changes, I've decided to go back to manually using after-change-functions and before-change-functions. track-changes.el showed promise: - One of the selling points was to turn complicated a-c-functions and b-c-functions into something easier, but that objectively didn't pan out, with "virtual" positions, one-shot hooks, and tracker registrations being abstractions and complications mastered by very few. - The other selling point was the ability to log and detect those parts of Emacs that cheat the modification hooks and correct them. As far as I can tell, only one such cheater -- quail.el -- was identified. But with little consequence, only an ugly workaround in eglot.el (now removed). - After using Eglot daily for all this time, I didn't notice any decrease in desynchronization events. - I did notice an increase in track-changes.el related bugs, some of which still baffle me and and hard to reproduce. A common occurence is the '(cl-assertion-failed (memq id track-changes--trackers))' which is hard to track down. - The library makes it more complicated to run Eglot on older Emacsen. I might yet revisit this matter for the next version but this experience has shown that it didn't bring the advantages I thought it would, so I'm abandoning it until at least 1.19 is out. * lisp/progmodes/eglot.el (track-changes): No longer require. (eglot--virtual-pos-to-lsp-position): Delete. (eglot--managed-mode): Simplify. (eglot--track-changes): Delete this variable. (eglot--recent-changes): Reword doc. (eglot--before-change, eglot--after-change): Bring back. (eglot--track-changes-fetch): Delete. (eglot--add-one-shot-hook): Delete. (eglot--track-changes-signal): Delete. (cherry picked from commit ac902ddadcd236dfd1d610768569e26ea8fc5b7f) --- diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index 1e2cca2186a..19412dc6158 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -7,7 +7,7 @@ ;; Maintainer: João Távora ;; URL: https://github.com/joaotavora/eglot ;; Keywords: convenience, languages -;; Package-Requires: ((emacs "26.3") (compat "27.1") (eldoc "1.14.0") (external-completion "0.1") (flymake "1.2.1") (jsonrpc "1.0.24") (project "0.9.8") (seq "2.23") (track-changes "1.2") (xref "1.6.2")) +;; Package-Requires: ((emacs "26.3") (compat "27.1") (eldoc "1.14.0") (external-completion "0.1") (flymake "1.2.1") (jsonrpc "1.0.24") (project "0.9.8") (seq "2.23") (xref "1.6.2")) ;; This is a GNU ELPA :core package. Avoid adding functionality ;; that is not available in the version of Emacs recorded above or any @@ -108,7 +108,6 @@ (require 'text-property-search nil t) (require 'diff-mode) (require 'diff) -(require 'track-changes) (require 'compat) ;; These dependencies are also GNU ELPA core packages. Because of @@ -1787,24 +1786,6 @@ LBP defaults to `eglot--bol'." :character (progn (when pos (goto-char pos)) (funcall eglot-current-linepos-function))))) -(defun eglot--virtual-pos-to-lsp-position (pos string) - "Return the LSP position at the end of STRING if it were inserted at POS." - (eglot--widening - (goto-char pos) - (forward-line 0) - ;; LSP line is zero-origin; Emacs is one-origin. - (let ((posline (1- (line-number-at-pos nil t))) - (linebeg (buffer-substring (point) pos)) - (colfun eglot-current-linepos-function)) - ;; Use a temp buffer because: - ;; - I don't know of a fast way to count newlines in a string. - ;; - We currently don't have `eglot-current-linepos-function' for strings. - (with-temp-buffer - (insert linebeg string) - (goto-char (point-max)) - (list :line (+ posline (1- (line-number-at-pos nil t))) - :character (funcall colfun)))))) - (defvar eglot-move-to-linepos-function #'eglot-move-to-utf-16-linepos "Function to move to a position within a line reported by the LSP server. @@ -2011,8 +1992,6 @@ For example, to keep your Company customization, add the symbol "A hook run by Eglot after it started/stopped managing a buffer. Use `eglot-managed-p' to determine if current buffer is managed.") -(defvar-local eglot--track-changes nil) - (define-minor-mode eglot--managed-mode "Mode for source buffers managed by some Eglot project." :init-value nil :lighter nil :keymap eglot-mode-map :interactive nil @@ -2026,10 +2005,8 @@ Use `eglot-managed-p' to determine if current buffer is managed.") ("utf-8" (eglot--setq-saving eglot-current-linepos-function #'eglot-utf-8-linepos) (eglot--setq-saving eglot-move-to-linepos-function #'eglot-move-to-utf-8-linepos))) - (unless eglot--track-changes - (setq eglot--track-changes - (track-changes-register - #'eglot--track-changes-signal :disjoint t))) + (add-hook 'after-change-functions #'eglot--after-change nil t) + (add-hook 'before-change-functions #'eglot--before-change nil t) (add-hook 'kill-buffer-hook #'eglot--managed-mode-off nil t) ;; Prepend "didClose" to the hook after the "nonoff", so it will run first (add-hook 'kill-buffer-hook #'eglot--signal-textDocument/didClose nil t) @@ -2064,6 +2041,8 @@ Use `eglot-managed-p' to determine if current buffer is managed.") (eldoc-mode 1)) (cl-pushnew (current-buffer) (eglot--managed-buffers (eglot-current-server)))) (t + (remove-hook 'after-change-functions #'eglot--after-change t) + (remove-hook 'before-change-functions #'eglot--before-change t) (remove-hook 'kill-buffer-hook #'eglot--managed-mode-off t) (remove-hook 'kill-buffer-hook #'eglot--signal-textDocument/didClose t) (remove-hook 'before-revert-hook #'eglot--signal-textDocument/didClose t) @@ -2093,10 +2072,7 @@ Use `eglot-managed-p' to determine if current buffer is managed.") (delq (current-buffer) (eglot--managed-buffers server))) (when (and eglot-autoshutdown (null (eglot--managed-buffers server))) - (eglot-shutdown server)))) - (when eglot--track-changes - (track-changes-unregister eglot--track-changes) - (setq eglot--track-changes nil))))) + (eglot-shutdown server))))))) (defun eglot--managed-mode-off () "Turn off `eglot--managed-mode' unconditionally." @@ -2687,7 +2663,7 @@ buffer." `(:triggerKind 2 :triggerCharacter ,trigger) `(:triggerKind 1))))) (defvar-local eglot--recent-changes nil - "Recent buffer changes as collected by `eglot--track-changes-fetch'.") + "Recent buffer changes as collected by `eglot--before-change'.") (cl-defmethod jsonrpc-connection-ready-p ((_server eglot-lsp-server) _what) "Tell if SERVER is ready for WHAT in current buffer." @@ -2695,59 +2671,63 @@ buffer." (defvar-local eglot--change-idle-timer nil "Idle timer for didChange signals.") +(defun eglot--before-change (beg end) + "Hook onto `before-change-functions' with BEG and END." + (when (listp eglot--recent-changes) + ;; Records BEG and END, crucially convert them into LSP + ;; (line/char) positions before that information is lost (because + ;; the after-change thingy doesn't know if newlines were + ;; deleted/added). Also record markers of BEG and END + ;; (github#259) + (push `(,(eglot--pos-to-lsp-position beg) + ,(eglot--pos-to-lsp-position end) + (,beg . ,(copy-marker beg nil)) + (,end . ,(copy-marker end t))) + eglot--recent-changes))) + (defvar eglot--document-changed-hook '(eglot--signal-textDocument/didChange) "Internal hook for doing things when the document changes.") -(defun eglot--track-changes-fetch (id) - (if (eq eglot--recent-changes :pending) (setq eglot--recent-changes nil)) - (track-changes-fetch - id (lambda (beg end before) - (cl-incf eglot--versioned-identifier) - (cond - ((eq eglot--recent-changes :emacs-messup) nil) - ((eq before 'error) (setf eglot--recent-changes :emacs-messup)) - (t (push `(,(eglot--pos-to-lsp-position beg) - ,(eglot--virtual-pos-to-lsp-position beg before) - ,(length before) - ,(buffer-substring-no-properties beg end)) - eglot--recent-changes)))))) - -(defun eglot--add-one-shot-hook (hook function &optional append local) - "Like `add-hook' but calls FUNCTION only once." - (let* ((fname (make-symbol (format "eglot--%s-once" function))) - (fun (lambda (&rest args) - (remove-hook hook fname local) - (apply function args)))) - (fset fname fun) - (add-hook hook fname append local))) - -(defun eglot--track-changes-signal (id &optional distance) - (cond - (distance - ;; When distance is <100, we may as well coalesce the changes. - (when (> distance 100) (eglot--track-changes-fetch id))) - (eglot--recent-changes nil) - ;; Note that there are pending changes, for the benefit of those - ;; who check it as a boolean. - (t (setq eglot--recent-changes :pending))) +(defun eglot--after-change (beg end pre-change-length) + "Hook onto `after-change-functions'. +Records BEG, END and PRE-CHANGE-LENGTH locally." + (cl-incf eglot--versioned-identifier) + (pcase (car-safe eglot--recent-changes) + (`(,lsp-beg ,lsp-end + (,b-beg . ,b-beg-marker) + (,b-end . ,b-end-marker)) + ;; github#259 and github#367: with `capitalize-word' & friends, + ;; `before-change-functions' records the whole word's `b-beg' and + ;; `b-end'. Similarly, when `fill-paragraph' coalesces two + ;; lines, `b-beg' and `b-end' mark end of first line and end of + ;; second line, resp. In both situations, `beg' and `end' + ;; received here seemingly contradict that: they will differ by 1 + ;; and encompass the capitalized character or, in the coalescing + ;; case, the replacement of the newline with a space. We keep + ;; both markers and positions to detect and correct this. In + ;; this specific case, we ignore `beg', `len' and + ;; `pre-change-len' and send richer information about the region + ;; from the markers. I've also experimented with doing this + ;; unconditionally but it seems to break when newlines are added. + (if (and (= b-end b-end-marker) (= b-beg b-beg-marker) + (or (/= beg b-beg) (/= end b-end))) + (setcar eglot--recent-changes + `(,lsp-beg ,lsp-end ,(- b-end-marker b-beg-marker) + ,(buffer-substring-no-properties b-beg-marker + b-end-marker))) + (setcar eglot--recent-changes + `(,lsp-beg ,lsp-end ,pre-change-length + ,(buffer-substring-no-properties beg end))))) + (_ (setf eglot--recent-changes :emacs-messup))) (when eglot--change-idle-timer (cancel-timer eglot--change-idle-timer)) - (setq eglot--change-idle-timer - (run-with-idle-timer - eglot-send-changes-idle-time nil - (lambda (buf) - (eglot--when-live-buffer buf - (when eglot--managed-mode - (if (track-changes-inconsistent-state-p) - ;; Not a good time (e.g. in the middle of Quail thingy, - ;; bug#70541): reschedule for the next idle period. - (eglot--add-one-shot-hook - 'post-command-hook - (lambda () - (eglot--when-live-buffer buf - (eglot--track-changes-signal id)))) - (run-hooks 'eglot--document-changed-hook) - (setq eglot--change-idle-timer nil))))) - (current-buffer)))) + (let ((buf (current-buffer))) + (setq eglot--change-idle-timer + (run-with-idle-timer + eglot-send-changes-idle-time + nil (lambda () (eglot--when-live-buffer buf + (when eglot--managed-mode + (run-hooks 'eglot--document-changed-hook) + (setq eglot--change-idle-timer nil)))))))) (defvar-local eglot-workspace-configuration () "Configure LSP servers specifically for a given project. @@ -2851,7 +2831,6 @@ When called interactively, use the currently active server" (defun eglot--signal-textDocument/didChange () "Send textDocument/didChange to server." - (eglot--track-changes-fetch eglot--track-changes) (when eglot--recent-changes (let* ((server (eglot--current-server-or-lose)) (sync-capability (eglot-server-capable :textDocumentSync)) @@ -2877,7 +2856,6 @@ When called interactively, use the currently active server" (defun eglot--signal-textDocument/didOpen () "Send textDocument/didOpen to server." ;; Flush any potential pending change. - (eglot--track-changes-fetch eglot--track-changes) (setq eglot--recent-changes nil eglot--versioned-identifier 0 eglot--TextDocumentIdentifier-cache nil)