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#70541: track-changes-mode logs warnings (with input method, in Eglot buffer) Date: Sun, 05 May 2024 13:48:50 -0400 Message-ID: References: <86ttjr2pzw.fsf@gnu.org> <86edau3gyy.fsf@gnu.org> <8634ra36ny.fsf@gnu.org> <861q6ou2cs.fsf@gnu.org> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="8454"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Richard Copley , 70541-done@debbugs.gnu.org, Eli Zaretskii To: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun May 05 19:49:59 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 1s3fzr-0001zy-1q for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 05 May 2024 19:49:59 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1s3fzY-0006yw-9I; Sun, 05 May 2024 13:49:40 -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 1s3fzW-0006xe-Pq for bug-gnu-emacs@gnu.org; Sun, 05 May 2024 13:49:38 -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 1s3fzW-0004fR-F0 for bug-gnu-emacs@gnu.org; Sun, 05 May 2024 13:49:38 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1s3fzt-0002Gu-R2 for bug-gnu-emacs@gnu.org; Sun, 05 May 2024 13:50:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 05 May 2024 17:50:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70541 X-GNU-PR-Package: emacs Original-Received: via spool by 70541-done@debbugs.gnu.org id=D70541.17149313738720 (code D ref 70541); Sun, 05 May 2024 17:50:01 +0000 Original-Received: (at 70541-done) by debbugs.gnu.org; 5 May 2024 17:49:33 +0000 Original-Received: from localhost ([127.0.0.1]:60986 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1s3fzR-0002Ga-68 for submit@debbugs.gnu.org; Sun, 05 May 2024 13:49:33 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:38679) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1s3fzL-0002GU-Dh for 70541-done@debbugs.gnu.org; Sun, 05 May 2024 13:49:31 -0400 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 8ECBB80013; Sun, 5 May 2024 13:48:57 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1714931332; bh=4BWQKGJD7B8np1X3LFYeEPd3krLOx3qulVcExU1fPjM=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=m0qARY/CpXW/GjmWhTIdCF9KPt+BvVaZRSxmLiuX9E8hIBJJNRysEHq16S8Vig6Cm NP/W9r4tUetdJTSGCq+iqnmKHS2H+IbL/TiTMmi94W0Z5Ou1dSWS7zz/VOApor3xsK wp0T+YpCWjOHBzaR7HFiyJqKRjgeo6Ig3YJ+mlb7iIR+0jWRdrX5yXgDPcX9IpCEhe tyQD9O43TehdGcs4WvH09j6Lg0zjcuvogs41Bw0zSr9gj+OpLh9Dv0zKhAWzJ4LykM s1/rLtGyak9T29KAwzCliXoEzK1MTEaZ4Wd6bnpbuFsvO+fT4JrAnLuHIiIz+qJOM8 SWxATK7BAtDcQ== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 0242180799; Sun, 5 May 2024 13:48:52 -0400 (EDT) Original-Received: from alfajor (unknown [45.72.201.215]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id C0E2D120280; Sun, 5 May 2024 13:48:51 -0400 (EDT) In-Reply-To: ("=?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?="'s message of "Sun, 5 May 2024 17:10:31 +0100") 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:284530 Archived-At: > I don't know that this is a "performance bug". I mean, misleading the > server, crashing it is only such a thing if one squints very hard. =F0=9F=99=82 > So while may be "significantly simpler" solution isn't in the cards, > I still think some simplification can happen (I don't understand > why post-command-hook needs to be used for example). Here's the reason I used `post-command-hook`: As you know, the affected code is run from an idle timer, and the change I introduced is to try and detect when the timer is run at an "inconvenient time". So my code delays the execution to "later". In general calling `run-with-idle-timer` from an idle timer is a bit tricky: if the wait time is smaller (or equal) than the current timer, then the new timer will fire immediately. Furthermore, in order for the buffer state to change, we generally want to wait for the end of the current idle time, so we want to fire a new timer that will run at the *next* idle time. I used `post-command-hook` to wait for the end of the current idle time. Arguably, we could/should move this trick to `timer.el` to provide a `run-with-idle-timer-but-not-now` kind of function. But AFAICT even using "insider info" we'd still have to use a contraption like `post-command-hook`, or otherwise we'd have to make more pervasive changes to the way timers are implemented. =F0=9F=99=81 > Anyway, I would need to see the quil failure scenario encoded as a > test in eglot-tests.el so I can have a clear shot at it. Should be > possible, no? Possible yes, but probably not very easy. You can probably start with (insert "a") (with-silent-modifications (insert "^") (sit-for (* 2 eglot-send-changes-idle-time)) (delete-region (1- (point)) (point))) The first `insert` is there to make sure Eglot is told that there's been some change, so it arms the idle-timer, and then the insert+wait+delete mim= ics the Quail behavior. > Thanks. That's great. It would also help to document your new > eglot--track-changes-fetch. I'm afraid I've lost track of how the > code is supposed to work after track-changes.el > > * why is :emacs-messup still a thing? Because `track-changes` can't hide the errors. The only thing it can do is do the detection for you and send you a "clean" error indication. =F0= =9F=99=81 > By the way did :emacs-messup solve this very problem too via > a full resync? Yes/no. With the old code, I think in the Quail situation your code would *not* have detected the inconsistency (so no `:emacs-messup`) and it would have occasionally sent inconsistent info the LSP server. `track-changes` works a tiny bit harder to detect inconsistencies, so in this Quail case it does detect it, which means it sends `:error` which Eglot turns into `:emacs-messup` which means the LSP server receives consistent info *but* it's costly and that info is equivalent to if the user had manually typed an "^" followed by deleting that "^" and replacing it with a `=E1=B5=83` (for example), so the LSP server will somet= imes see the buffer with a `^` char which the user may never have meant to include in the buffer. And with the new code, Eglot postpones sending the state of the buffer until we're not in the middle of a Quail insertion (so track-changes never sends a `:error`, we don't use any `:emacs-messup`, and the LSP server never sees that `^` either). [ "never" here applies only to the Quail case, there can still be cases where we get `:error`, sadly. ] > * what exactly does the local eglot--track-changes do and why would > it be re-registered in the same buffer (why isn't the typical add-hook > enough). Every client of `track-changes` can consume the changes at its own rhythm, so track-changes needs to keep track of which changes each client has already consumed. > * There seems to be a track-changes.el signalling function and the > longstanding LSP signalling function that informs the server of > things. Why can't it be the same function, i.e. why can't Eglot > tell track-changes.el to call Eglot's function when track-changes.el > knows it's a safe time to call such a function? The flow of > information via the eglot--recent-changes variable is complicated > and would seem not worth bringing in track-changes.el at all. > Is it only to accommodate Eglot versions when track-changes.el > isn't available or is there some deeper reason? We could turn `eglot--recent-changes` into a boolean, or probably even eliminate it altogether, yes. The reason I kept it is that if we don't have it, then all the changes since the last `eglot--signal-textDocument/didChange` get combined into a single change, which can become quite large if the changes are spread out. Stefan