From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Newsgroups: gmane.emacs.bugs Subject: bug#73857: [PATCH] * lisp/progmodes/eglot.el: add support for insertReplaceEdit Date: Sat, 2 Nov 2024 13:22:16 +0000 Message-ID: References: <86plnynecv.fsf@gnu.org> <86v7x5yhl8.fsf@gnu.org> 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="33599"; mail-complaints-to="usenet@ciao.gmane.io" Cc: kcbanner@gmail.com, 73857@debbugs.gnu.org To: Eli Zaretskii , Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Nov 02 14:22:49 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 1t7E5X-0008YW-TE for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 02 Nov 2024 14:22:48 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t7E5H-00055k-Ab; Sat, 02 Nov 2024 09:22:32 -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 1t7E4y-0004yZ-Gs for bug-gnu-emacs@gnu.org; Sat, 02 Nov 2024 09:22:19 -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 1t7E4o-0002fm-HQ for bug-gnu-emacs@gnu.org; Sat, 02 Nov 2024 09:22:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=Date:From:In-Reply-To:References:MIME-Version:To:Subject; bh=UxJlCRGraY6wGHWkjkpGb+QGQ08Epoj9Zshs9kXLwZg=; b=E0XK2+4g5peqGtZInL6cU6v/6eTmsq3VP9mvlN6Sf3YeGErcpmsdC1b64WCf40ByJZHEuic0vW+M3IJfhuZTA3DCWKY+kmqPb6IY8oHnV8AD+iyePFM1jY/hMW/MSBPNsAR1Muos9KPJtHjxv1KnDkN+cUW01PfCYJS3Cz31eO1oEy8XKhU3AKPhqa842mEJiX34gCGZvD0cEvaK5tAoABCjyWaLeLn6YcRalHkODMaqV0IY4lIE266Q4x8Ux2Qtfw1Bptv3RmjsPoLgDgbeTkBeYrhyPPWP6CAdgJZbMqYM4lJ6XlfmJKECAaoindpbrYWxpSG8h146sevSQqVLtQ==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1t7E4o-0002t7-2q for bug-gnu-emacs@gnu.org; Sat, 02 Nov 2024 09:22:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 02 Nov 2024 13:22:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73857 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 73857-submit@debbugs.gnu.org id=B73857.173055370811085 (code B ref 73857); Sat, 02 Nov 2024 13:22:02 +0000 Original-Received: (at 73857) by debbugs.gnu.org; 2 Nov 2024 13:21:48 +0000 Original-Received: from localhost ([127.0.0.1]:53495 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t7E4Z-0002sj-Lf for submit@debbugs.gnu.org; Sat, 02 Nov 2024 09:21:48 -0400 Original-Received: from mail-oo1-f53.google.com ([209.85.161.53]:47344) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t7E4V-0002sa-8c for 73857@debbugs.gnu.org; Sat, 02 Nov 2024 09:21:45 -0400 Original-Received: by mail-oo1-f53.google.com with SMTP id 006d021491bc7-5ebc22e6362so1312523eaf.2 for <73857@debbugs.gnu.org>; Sat, 02 Nov 2024 06:21:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730553637; x=1731158437; darn=debbugs.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=UxJlCRGraY6wGHWkjkpGb+QGQ08Epoj9Zshs9kXLwZg=; b=OMYPM7jzisPl6lwTGQJ9BwjcVoBzfwxLMjObr6+rOpDaejXjJjqRDy1cQgflQytWKJ hPYCR3tw7MJMsjDINLzoIgunEA7z+n/fpy168tHcjC+gRWQ5WRXXDVUQqin20YlwWZvO ejvSO8FFj9W1gl/HVDtZhMjXFDnLCBzTvoi2NGTiuu5kZpmFW5/t4Q/5+8NUHM8DdJC1 WtTWsozlzHviI10o8Iw+Hi/aDsEy5uJbbe9IIZ8OizUd296Qa7H3knYLnacdh/xPhyG1 scHE2R43+OUavdd146opNGLpTU8rBdIZfLP6a+Cj+oouDBPkjyvp48TvcojGdvk4NLjK ELJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730553637; x=1731158437; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=UxJlCRGraY6wGHWkjkpGb+QGQ08Epoj9Zshs9kXLwZg=; b=Xe8X+0gdF3tWhP9SaHaif9R1TNZWxxk0UBl+rZfpkAEZUIxP4fs8o5kxmcKy7qin4x EthbUsYjbsl4YcMGQI6WhNG6aZ/u7vnyGrT0UL3WSLliLhmCWEE3/IzA6gk0QcLaY0I7 ca4YeZpZAcg8P2P7RGgWhIFUFPvP9CKPcj3jaXIzWOnjcqIptOXyLw07LuyeKrRSMPEI 8UWL8T+WIF6GZAt6T0dN9G18qaUPELMoeLg6U41QbBA6mDXkH0IvIMpvvPgPibCPFodd SS9dUACjSkXhvvf4W33aDNymCf/CLxx8x1Qtg3RTQKM35v6FYOzE+bq2Le/0h3V9CV2g UUVg== X-Forwarded-Encrypted: i=1; AJvYcCUnSIH8jKsPbxgIq9IWXFNpyhMPRmphMAoeo6IoKNRNHJeuaXihJFu4fetBrsBRW4kDrSLudA==@debbugs.gnu.org X-Gm-Message-State: AOJu0YzLSVkFD8Tati7NgCKphHxQ+vqkoOGXGQi1le1Zfenrtg36a5SZ b8C5hn7l2EI2bMq4THf/ttE7ZgsZPgEhWcOE97RyK7xaSJQ5STwW1Q7gDOhw7IedFvXIu003y+L gvdnOw2NjJEAk4HqLErcyE7Why40= X-Google-Smtp-Source: AGHT+IGZ+D3IZeVllS3devgD080R9Qh5xavkyxrzEL5ZLm0ooAvvoBRYxBi3WTKSDlp5l0HTHOGabzeVPy0JnqjBXf8= X-Received: by 2002:a05:6820:1c9d:b0:5e5:941c:ca5a with SMTP id 006d021491bc7-5ec6da771ebmr6573326eaf.1.1730553637361; Sat, 02 Nov 2024 06:20:37 -0700 (PDT) In-Reply-To: <86v7x5yhl8.fsf@gnu.org> 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:294745 Archived-At: First, the patch looks good. I haven't been able to test it, but it looks good, very good even. The issue seems to be valid, and I've confirmed it, but I don't fully understand it. Here, I find it somewhat dissatisfying that what would seem to be a client-side issue (i.e. a bug in Eglot) should be "fixed" by adding a feature so as to somehow circumvent the problem. But another possible explanation is that the issue is not a bug in Eglot but a bug in certain servers (such as rust-analyzer) which given the reasonable/common absence of a capability in the client (the one that's being proposed by Casey) utilizes Eglot's current capabilities in a suboptimal way that triggers a problem. In that case -- which would have to be confirmed -- fixing the server or adding the capability to the client are two equally valid alternatives to solve this. We seem to have the latter in the patch. When I find the time I will try to test this patch, unless someone beats me to it with a convincing test (even better a convincing automated test in eglot-tests.el). Such a person could be Dmitry, who has worked on Eglot completion logic before. I've added him to CC. Jo=C3=A3o On Sat, Nov 2, 2024 at 11:56=E2=80=AFAM Eli Zaretskii wrote: > > Ping! Jo=C3=A3o, any comments to the patch or the issue in general? > > > Cc: 73857@debbugs.gnu.org > > Date: Fri, 18 Oct 2024 08:56:32 +0300 > > From: Eli Zaretskii > > > > > From: Casey Banner > > > Date: Thu, 17 Oct 2024 20:54:38 -0400 > > > > > > Since 3.16, LSP supports the capability `insertReplaceSupport`. This > > > allows textEdit to be an `InsertReplaceEdit` see: > > > (https://microsoft.github.io/language-server-protocol/specifications/= lsp/3.17/specification/#insertReplaceEdit) > > > > > > This patch adds support for this capability, and uses the `replace` > > > field of the `InsertReplaceEdit`. Original functionality (ie. > > > `TextEdit`) is preserved. > > > > > > The benefits of this were originally discussed here: > > > https://github.com/joaotavora/eglot/discussions/1456, but this is a s= ummary: > > > > > > Consider this file: > > > > > > ``` > > > const Foo =3D struct { > > > correct_name: u32, > > > }; > > > > > > fn example(foo: Foo) u32 { > > > return foo.correct_name; > > > } > > > ``` > > > > > > 1. Place the cursor on 6:22 (the _ in correct_name) > > > 2. Backspace once to delete the t > > > 3. Receive the following LSP message: `<-- textDocument/completion[20= ] {"jsonrpc":"2.0","id":20,"result": > > > {"isIncomplete":false,"items":[{"label":"correct_name","kind":5,"deta= il":"u32","documentation": > > > {"kind":"plaintext","value":""},"sortText":"2_correct_name","textEdit= ":{"range":{"start": > > > {"line":5,"character":15},"end":{"line":5,"character":21}},"newText":= "correct_name"}}]}}` > > > 4. Accept the completion > > > 5. The buffer now contains ` return foo.correct_name_name;` on lin= e 6 > > > > > > I expected it to replace the entire token, resulting in ` return f= oo.correct_name;`. > > > > > > Indeed with this patch applied (and an LSP that supports the > > > capability), the behaviour I expected is now what happens. > > > > > > This is the first real elisp that I've written besides configuration,= so > > > I'm not sure if this is the correct way, but it seems to work for me. > > > > > > Patch is attached. > > > > Thanks. > > > > Jo=C3=A3o, any comments? > > > > > From bbf79f95636d699ccf9ba7028e6c3dce23af2378 Mon Sep 17 00:00:00 200= 1 > > > From: kcbanner > > > Date: Thu, 17 Oct 2024 00:43:32 -0400 > > > Subject: [PATCH] * lisp/progmodes/eglot.el: add support for insertRep= laceEdit > > > > > > --- > > > lisp/progmodes/eglot.el | 25 +++++++++++++++++++------ > > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > > > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el > > > index 0a14146a245..8285506928f 100644 > > > --- a/lisp/progmodes/eglot.el > > > +++ b/lisp/progmodes/eglot.el > > > @@ -647,6 +647,7 @@ eglot--uri-path-allowed-chars > > > (:detail :deprecated :children)) > > > (TextDocumentEdit (:textDocument :edits) ()) > > > (TextEdit (:range :newText)) > > > + (InsertReplaceEdit (:newText :insert :replace)) > > > (VersionedTextDocumentIdentifier (:uri :version) ()) > > > (WorkDoneProgress (:kind) (:title :message :percentage :cancel= lable)) > > > (WorkspaceEdit () (:changes :documentChanges)) > > > @@ -959,7 +960,8 @@ eglot-client-capabilities > > > ["documentati= on" > > > "details" > > > "additionalT= extEdits"]) > > > - :tagSupport (:valueSet [1])) > > > + :tagSupport (:valueSet [1]) > > > + :insertReplaceSupport t) > > > :contextSupport t) > > > :hover (list :dynamicRegistration :json-fa= lse > > > :contentFormat (eglot--accept= ed-formats)) > > > @@ -3368,12 +3370,19 @@ eglot-completion-at-point > > > ;; state, _not_ the current "foo.bar". > > > (delete-region orig-pos (point)) > > > (insert (substring bounds-string (- orig-pos= (car bounds)))) > > > - (eglot--dbind ((TextEdit) range newText) tex= tEdit > > > - (pcase-let ((`(,beg . ,end) > > > + (eglot--dcase textEdit > > > + (((TextEdit) range newText) > > > + (pcase-let ((`(,beg . ,end) > > > (eglot-range-region range))) > > > (delete-region beg end) > > > (goto-char beg) > > > - (funcall (or snippet-fn #'insert) newTex= t)))) > > > + (funcall (or snippet-fn #'insert) newTex= t))) > > > + (((InsertReplaceEdit) newText replace) > > > + (pcase-let ((`(,beg . ,end) > > > + (eglot-range-region replace)= )) > > > + (delete-region beg end) > > > + (goto-char beg) > > > + (funcall (or snippet-fn #'insert) newTe= xt))))) > > > (snippet-fn > > > ;; A snippet should be inserted, but using p= lain > > > ;; `insertText'. This requires us to delete= the > > > @@ -3602,8 +3611,12 @@ eglot--apply-text-edits > > > (replace-buffer-contents temp))) > > > (when reporter > > > (eglot--reporter-update reporter (cl-incf do= ne)))))))) > > > - (mapcar (eglot--lambda ((TextEdit) range newText) > > > - (cons newText (eglot-range-region range 'marke= rs))) > > > + (mapcar (lambda (text-edit-or-insert-replace-edit) > > > + (eglot--dcase text-edit-or-insert-replace-edit > > > + (((TextEdit) range newText) > > > + (cons newText (eglot-range-region range 'ma= rkers))) > > > + (((InsertReplaceEdit) newText replace) > > > + (cons newText (eglot-range-region replace '= markers))))) > > > (reverse edits))) > > > (undo-amalgamate-change-group change-group) > > > (when reporter > > > -- > > > 2.46.0.windows.1 > > > > > > > > > > > -- Jo=C3=A3o T=C3=A1vora