From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#70077: An easier way to track buffer changes Date: Sat, 30 Mar 2024 01:09:41 -0400 Message-ID: References: <86frw8ewk9.fsf@gnu.org> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="21859"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: casouri@gmail.com, 70077@debbugs.gnu.org, yantar92@posteo.net, qhong@alum.mit.edu, frederic.bour@lakaban.net, joaotavora@gmail.com, mail@nicolasgoaziou.fr, acm@muc.de, stephen_leake@stephe-leake.org, alan.zimm@gmail.com, phillip.lord@russet.org.uk To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Mar 30 06:10:32 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rqQz9-0005RG-LO for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 30 Mar 2024 06:10:31 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rqQyi-0002WF-IB; Sat, 30 Mar 2024 01:10:04 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rqQyf-0002Vs-6H for bug-gnu-emacs@gnu.org; Sat, 30 Mar 2024 01:10:01 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rqQye-0000OO-Nd for bug-gnu-emacs@gnu.org; Sat, 30 Mar 2024 01:10:00 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rqQyg-0002M1-3P for bug-gnu-emacs@gnu.org; Sat, 30 Mar 2024 01:10:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 30 Mar 2024 05:10:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70077 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 70077-submit@debbugs.gnu.org id=B70077.17117753969021 (code B ref 70077); Sat, 30 Mar 2024 05:10:02 +0000 Original-Received: (at 70077) by debbugs.gnu.org; 30 Mar 2024 05:09:56 +0000 Original-Received: from localhost ([127.0.0.1]:43719 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rqQyZ-0002LR-Cd for submit@debbugs.gnu.org; Sat, 30 Mar 2024 01:09:56 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:59441) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rqQyW-0002LD-K8 for 70077@debbugs.gnu.org; Sat, 30 Mar 2024 01:09:53 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id A19931000DD; Sat, 30 Mar 2024 01:09:44 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1711775383; bh=oPJpqrXJloZ+Vj4qPyoHlTphSd1+E1YAC3BhOuiPyEE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=JPZUl3tTNIaiVEUoULQkFrWwld2rfnexERFYiYM2yFcZwEqworokVaioxP0ulJwu2 dMD/9kLTrlDVYqPX7G3cglRGMKDGXdQyxNvBRge5R7MFKyAoVZNuTJfEhnb9ANEgWe GDaQtqfb5iUJGXaK7XNhfHQFTr3T7QdcQURwmmhwKtJVnc+aXxFt6Ampd0tgJLQehA LwCnynhj49PqnhSRJH7wru9Dxpmgs8PPXKG6IRHrB/PQ03KbEuNTmb4KxHjMfdPU2B BXmDcoc3t0YDEl/ZyJzqb7VdPfXeqNSg/kQLRYcsnpJz5S0nrPWjWUCMgaAj3Ud6xU jedR4aQWpusIA== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 460A210004A; Sat, 30 Mar 2024 01:09:43 -0400 (EDT) Original-Received: from pastel (104-222-113-60.cpe.teksavvy.com [104.222.113.60]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id E3186120642; Sat, 30 Mar 2024 01:09:42 -0400 (EDT) In-Reply-To: (Stefan Monnier's message of "Fri, 29 Mar 2024 23:17:09 -0400") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:282322 Archived-At: --=-=-= Content-Type: text/plain > Here's my first attempt at a real-life use. And here's a second attempt, which is a tentative patch for `eglot.el`. This one does make use of the `before` argument, so it exercises more the API. The `eglot--virtual-pos-to-lsp-position` is not completely satisfactory, since to compute the LSP position of the end of the chunk before it was modified, I end up creating a temp buffer to insert the part of the text that changed (to count its line+column, which is much easier in a buffer than in a string). That kinda sucks performancewise, but we do it at most once per command rather than once per buffer-modification, so it should be lost in the noise. The upside is that we're insulated from the quirks of the after/before-change-functions evidenced by the copious comments referring to various bug reports. Stefan --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=eglot.patch diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index 7d2f1a55165..d2268cea940 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -110,6 +110,7 @@ (require 'text-property-search nil t) (require 'diff-mode) (require 'diff) +(require 'track-changes) ;; These dependencies are also GNU ELPA core packages. Because of ;; bug#62576, since there is a risk that M-x package-install, despite @@ -1732,6 +1733,9 @@ eglot-utf-16-linepos "Calculate number of UTF-16 code units from position given by LBP. LBP defaults to `eglot--bol'." (/ (- (length (encode-coding-region (or lbp (eglot--bol)) + ;; FIXME: How could `point' ever be + ;; larger than `point-max' (sounds like + ;; a bug in Emacs). ;; Fix github#860 (min (point) (point-max)) 'utf-16 t)) 2) @@ -1749,6 +1753,24 @@ eglot--pos-to-lsp-position :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-char 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. @@ -1946,6 +1968,8 @@ eglot-managed-mode-hook "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 @@ -1959,8 +1983,9 @@ eglot--managed-mode ("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))) - (add-hook 'after-change-functions #'eglot--after-change nil t) - (add-hook 'before-change-functions #'eglot--before-change nil t) + (unless eglot--track-changes + (setq eglot--track-changes + (track-changes-register #'eglot--track-changes-signal))) (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) @@ -1994,8 +2019,8 @@ eglot--managed-mode (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) + (when eglot--track-changes + (track-changes-unregister eglot--track-changes)) (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) @@ -2564,54 +2589,20 @@ jsonrpc-connection-ready-p (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--after-change (beg end pre-change-length) - "Hook onto `after-change-functions'. -Records BEG, END and PRE-CHANGE-LENGTH locally." +(defun eglot--track-changes-signal (id) (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))) + (track-changes-fetch + id (lambda (beg end before) + (if (stringp before) + (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) + (setf eglot--recent-changes :emacs-messup)))) (when eglot--change-idle-timer (cancel-timer eglot--change-idle-timer)) (let ((buf (current-buffer))) (setq eglot--change-idle-timer @@ -2741,12 +2732,6 @@ eglot--signal-textDocument/didChange (buffer-substring-no-properties (point-min) (point-max))))) (cl-loop for (beg end len text) in (reverse eglot--recent-changes) - ;; github#259: `capitalize-word' and commands based - ;; on `casify_region' will cause multiple duplicate - ;; empty entries in `eglot--before-change' calls - ;; without an `eglot--after-change' reciprocal. - ;; Weed them out here. - when (numberp len) vconcat `[,(list :range `(:start ,beg :end ,end) :rangeLength len :text text)])))) (setq eglot--recent-changes nil) --=-=-=--