From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Richard Copley Newsgroups: gmane.emacs.bugs Subject: bug#66552: 30.0.50; Eglot feature request: handle quirky code actions Date: Wed, 18 Oct 2023 00:24:21 +0100 Message-ID: References: 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="33254"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 66552@debbugs.gnu.org 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 Wed Oct 18 01:26:05 2023 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 1qstRs-0008RZ-Ux for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 18 Oct 2023 01:26:05 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qstRS-000651-Tb; Tue, 17 Oct 2023 19:25:38 -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 1qstRR-00064r-7P for bug-gnu-emacs@gnu.org; Tue, 17 Oct 2023 19:25:37 -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 1qstRQ-0005l0-UM for bug-gnu-emacs@gnu.org; Tue, 17 Oct 2023 19:25:36 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qstRq-0000IR-9e for bug-gnu-emacs@gnu.org; Tue, 17 Oct 2023 19:26:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Richard Copley Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 17 Oct 2023 23:26:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 66552 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: notabug Original-Received: via spool by 66552-submit@debbugs.gnu.org id=B66552.16975851241083 (code B ref 66552); Tue, 17 Oct 2023 23:26:02 +0000 Original-Received: (at 66552) by debbugs.gnu.org; 17 Oct 2023 23:25:24 +0000 Original-Received: from localhost ([127.0.0.1]:32944 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qstRD-0000HN-J3 for submit@debbugs.gnu.org; Tue, 17 Oct 2023 19:25:24 -0400 Original-Received: from mail-qt1-x82e.google.com ([2607:f8b0:4864:20::82e]:55706) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qstR9-0000H7-GC for 66552@debbugs.gnu.org; Tue, 17 Oct 2023 19:25:21 -0400 Original-Received: by mail-qt1-x82e.google.com with SMTP id d75a77b69052e-4197bb0a0d9so40217121cf.3 for <66552@debbugs.gnu.org>; Tue, 17 Oct 2023 16:24:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1697585088; x=1698189888; 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=/rw+aR5Bqa4/EXVh3Qozj/wATy4TIXVg0zAH9hNoR/E=; b=Q/6rNAZIeE2HYxbrpecqWRHLdwafSnsVftT4EikMXr/46J1h8CYX3ANAacP6qCuqNu 0hW9Z/m5tda42eB/D3flmCqEVoBiXGnauNwydLquNd2ZKr/eNDumFS3zGPaWAl2rKNe7 mmxhm6auvzLSXB2PRS63exWSNMdomynIpsAuaqsRt5Pw7rJBwjP/fXbHxC924gb8m8Q2 HJBpsmSpjXCZndhN+L+D4ChXEqKdsgn5ZiY1VRCZZI0tG+TYFo40tHbEi/KzVf5NBLCS +/TeK5FlThPciaecXEw/mGI5R7LMXWxvPEkz26Ebi/dqSKK8tMnjycgXrdZArIIhl0dR DW1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697585088; x=1698189888; 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=/rw+aR5Bqa4/EXVh3Qozj/wATy4TIXVg0zAH9hNoR/E=; b=KXNEfn8Qfky/gQ9kc38pbgMnShNz2RCH7EjjhZSDyBfLc+c2G9rinfBizCAZY8OSHT ZKWuxQL8R0uk8xtr4oBxlWmHLH2kePZXOJ4fTvLVAEjmgk2lzBETdsqjW3N5ZUWWezqc N6J9AyOtEJ4SKx3HciNqMkLn2mBvRASsQqd5Qkft506gMep6QweC9tjxPvVEhYpXZx0q JPFLorxloy9xeJEsVTFNMkE/4bd1nw2ibU20rtEUnMVivDuVDI0T/vGaImNQf4iI4Txz R9Vr84fpuRbxZMq3fPNWcK916ICDqWbLb+aspyvNrwwcAPXnUuPqp4tme2DK22/R3fij AmMA== X-Gm-Message-State: AOJu0Yx2unYAiF5d1MwPeKvLBXKEge5zJ8U0pcs3gKOyPzhFQ6XHfmvm mu/xrjjFJzJU/zenWhSSE7/8ApNfZsPmx1jV9dg= X-Google-Smtp-Source: AGHT+IE3j1MmeVA1cLgwgkah3i4Ev2pKG3h3GyAmfRuiXzlYDfTVKono50px5TBSlvzuBZXZHL7MHi+FUkyZ+hfO9oI= X-Received: by 2002:a05:622a:1190:b0:419:a2c6:8207 with SMTP id m16-20020a05622a119000b00419a2c68207mr3963453qtk.22.1697585088014; Tue, 17 Oct 2023 16:24:48 -0700 (PDT) In-Reply-To: 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:272640 Archived-At: On Tue, 17 Oct 2023 at 22:38, Jo=C3=A3o T=C3=A1vora = wrote: > > On Tue, Oct 17, 2023 at 8:08=E2=80=AFPM Richard Copley wrote: > > > > If a servers sends no field, Eglot will > > > ignore it. If it sends a wrong one, Eglot won't. > > > > > You could work around it with a monkey-patch by advising > > > eglot--apply-text-edits to always have a null second argument. > > > > I'll probably go with that. Thanks for the suggestion. It's a shame > > that this will affect all languages. It would be nice if I could use a > > defmethod dispatching on a subclass of eglot-lsp-server! But > > eglot--apply-text-edits doesn't take a server, so it isn't generic. > > For eglot--apply-text-edits to become a generic, it would first > have to be promoted to Eglot's user API (losing the --), and I > don't see a very good reason to do that right now. > > However, if you wanted a fully "legal" solution, I think you could > place a server-specific method on > > eglot-handle-request (server your-server) (method workspace/applyEdit) > > which removes or massages the `version` cookie in the `edit` keyword > argument. In my case the reply to the textDocument/codeAction client request contains the full WorkspaceEdit, so there is no workspace/applyEdit server request involved. The edits are applied directly (eglot-code-actions (interactive) -> eglot--read-execute-code-action -> eglot-execute -> eglot--apply-workspace-edit). I'm using :before advice on eglot--read-execute-code-action to massage the list of actions (both the version number and the title text). > And even with the monkey-patching approach, you can make something > server specific by checking the return value of `eglot-current-server` > before tweaking the value. I will do that, thanks. > Another possibility is for Eglot to start interpreting version 0 as > "any version". It should be feasible since Eglot controls the > start of the numbering, which right now is 0 but be increased to 1, > hopefully without any negative consequences. > > > > The second labeling problem is even more bizarre Again, you > > > can probably monkey-patch Eglot to work around it. > > > > Here I'm not sure I agree. The title is only specified to be "A short, > > human-readable, title for this code action". In this case the > > suggestions are computed, and inventing unique friendly names for them > > isn't feasible. "Apply 'Try this'" seems sensible. It refers to the > > text of the corresponding diagnostic, "Try this : ". > > This newText is hidden deep inside the `edits` field of such messages. > There's no guarantee it's a 1-element array or even that the edit > is just an insertion. So grabbing it consistently to craft a > user-visible message is just a bad idea That's true. > It doesn't make sense for a language server to provide a bunch > of actions with identical labels. It isn't something the client > should have to deal with. A label, to me, is a piece of information > for telling one object apart from another object. If the text of the > corresponding diagnostic is (presumably unique), why can't > the label be unique as well? But these aren't labels, they're titles, and given the specification, the authors providing code actions won't necessarily foresee their use as labels, as sensible as it is from the client's point of view. The message from the diagnostics *could* be used as the title, but I'd find it hard to argue to the developers that it *should*. That's no help to you, of course! I agree that my suggestion of building a label from the newText is a non-starter in the general case. For the time being it will work for my case, but if Lean 4 starts providing refactoring support, for example, I'll have to think again.