From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh Newsgroups: gmane.emacs.bugs Subject: bug#71504: 30.0.50; FR: Fix suggestions ("quick fix") for Flymake diagnostics Date: Wed, 24 Jul 2024 12:40:32 -0400 Message-ID: References: <865xtu7seh.fsf@gnu.org> <868qyf5587.fsf@gnu.org> <868qyd32ih.fsf@gnu.org> <861q452s5f.fsf@gnu.org> <86frsgmpe1.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="16058"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Eli Zaretskii , 71504@debbugs.gnu.org, joaotavora@gmail.com To: Eshel Yaron Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Jul 24 18:41:15 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 1sWf3C-0003yq-Bq for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 24 Jul 2024 18:41:14 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sWf2v-0005Yt-Lb; Wed, 24 Jul 2024 12:40:57 -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 1sWf2u-0005Yk-JW for bug-gnu-emacs@gnu.org; Wed, 24 Jul 2024 12:40:56 -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 1sWf2u-0006LH-By for bug-gnu-emacs@gnu.org; Wed, 24 Jul 2024 12:40:56 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sWf30-0004ig-7o for bug-gnu-emacs@gnu.org; Wed, 24 Jul 2024 12:41:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 24 Jul 2024 16:41:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 71504 X-GNU-PR-Package: emacs Original-Received: via spool by 71504-submit@debbugs.gnu.org id=B71504.172183924818109 (code B ref 71504); Wed, 24 Jul 2024 16:41:02 +0000 Original-Received: (at 71504) by debbugs.gnu.org; 24 Jul 2024 16:40:48 +0000 Original-Received: from localhost ([127.0.0.1]:34381 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sWf2l-0004i0-PB for submit@debbugs.gnu.org; Wed, 24 Jul 2024 12:40:48 -0400 Original-Received: from mxout5.mail.janestreet.com ([64.215.233.18]:50197) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sWf2i-0004hk-Sw for 71504@debbugs.gnu.org; Wed, 24 Jul 2024 12:40:46 -0400 In-Reply-To: (Eshel Yaron's message of "Wed, 17 Jul 2024 13:51:27 +0200") DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1721839233; bh=ZOOVIT1kvIHw3MxXkekmS8zMX+6m0fn8T+kC8jzAfD4=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=UTN/xHYTI6iC7bmpJwjMleuIy/PUdmte+rrtLXhwWGakYTPL8vsQwlGylj9YA2tT1 3opwuzGFbbQyFjcZ3TK8DhhzqJsqAP8keEPSiVBTb057PlgiaraNQ12RP7O6dObwNM fR6gieC04uJ/L7VUqNKWDeJrKmjTnydt7/FlBGVwm//elvxm6A+8OYmVfrVTJjH3A1 GvQwaDD9+hva4WD0GOwzyKB6JppfXlu+XyXhXZIgCm1ZR19y8RuCAugoWrFkMHCjcH o1EyLcIksieFNMeZ/ZaLDiHSMclt5si5SuOZpBrDVQ+lQJbwEo3jIHDmiuY4UppsEM 1XXGIvBMMJRsA== 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:289240 Archived-At: Eshel Yaron writes: > Hi Spencer, > > Spencer Baugh writes: > >> Eshel Yaron writes: >>> Eli Zaretskii writes: >>>> >>>> I'm asking what is the overall idea of the proposed implementation. I >>>> think it's worthwhile to present it, so we could see if we all agree >>>> with that idea and the details of the proposed implementation. >>> >>> Thanks. To clarify, ideally Spencer will implement this feature request >>> however he sees fit. I'm offering my implementation as a reference, but >>> I'm not advocating for it over other alternatives that may come up. >>> >>> The idea of my implementation is to allow Flymake backends to associate >>> fixes with some of the diagnostics they create, and to add a command >>> that tries to apply a fix for the diagnostic at point. For the details, >>> see below the same patch I attached to this message: >>> https://lists.gnu.org/archive/html/emacs-devel/2024-05/msg01318.html >> >> A few issues: >> >> - At an immediate glance, fix-function should return a cl-defstruct >> instead of a list, to match the rest of the flymake API. > > The fix-function returns any number of fixes, as a list of fixes. I > guess that's not the list you're referring to, because I don't see how > this list (whose number of elements vary) can be replaced by a > cl-defstruct. Each individual fix is also represented as a list, with > exactly two elements: TITLE and EDITS. Is this what you'd like to see > replaced with a cl-defstruct? Yes. >> - An implementation of this feature in flymake absolutely must come with >> support for Eglot, as the main user of this feature. Which, if the >> Eglot maintainer doesn't want to merge that, may mean we can't move >> forward. > > I agree that Eglot support is important (it was naturally the first > backend I adapted), and that it'll be great to have Jo=C3=A3o on board. > I don't think it's a blocker, but it's your call. I don't want to be obstructionist, but it seems to me that if we land this in flymake, eventually it will just *have* to be supported in Eglot. I don't want to abuse that as a way to work around what Joao prefers. >> - Your patch adds support in shellcheck for fixes. That's >> uncontroversially useful and good. Could we add this support in a >> shellcheck-specific way before finalizing the flymake fix API? (Taking >> care to add it in a way that can be supported by a later unified UI, of >> course) > > Maybe, but it would be less useful: it wouldn't help other backends, > including third-party backends, to provide fixes. Also, I don't think > there's anything final here, there's plenty of room for developing the > API further if more/different requirements arise. Of course that's all true, but also it doesn't seem harmful to just add some shellcheck-apply-fix command, which contains all the shellcheck-specific logic you already need to add. It will have a tiny bit of duplication, but we can get rid of that after adding a unified UI. >> - Likewise, you mentioned adding support for fixes to checkdoc, although >> I'm not sure where that patch is. > > It's on my development branch, namely this commit: > > http://git.eshelyaron.com/gitweb/?p=3Demacs.git;a=3Dcommitdiff;h=3D650c2a= 056af8df85065b2851d3513c1e3d62c60c Is this server down right now? >> That sounds also extremely useful, could it also be added first? > > First as in not via Flymake? Note that checkdoc already has its own fix > support, the point here is to add something that'll work consistently > across backends. Ah, I'll have to judge when I see the commit. >> - Do you hope to have a default binding for the fix-applying command you >> mention? Certainly I would like that, and I think it's worth talking >> about now. > > IMO it's fine to leave it unbound at first, and see how users bind it. > But if you find an appropriate available binding, why not? :) > >> More broadly, I'm still a bit unsure about this whole approach. >> >> - What UI, exactly, do you want to build on top of this? Can we see an >> example of this UI? Or is it really just this one command? > > A simple command is a good start. Once diagnostics are enriched with > fix suggestions, folks can add more bells and whistles if they like to. > > FWIW, two further small enhancements that I'm experimenting with are: > 1. Apply fixes from the diagnostics list buffer. > 2. Apply fixes to one or more diagnostics that you choose with completion. > >> - If it's just the one command, and your hope is to give this some >> default binding, what exactly is the problem with doing that via a >> keymap overlay as Joao suggests? What do you want to do which can't be >> done with a keymap overlay bound to a backend-specific function? > > As we've already established, individual backends can do anything at all > via overlay properties and other methods. But that puts extraneous > burden on backends to deal with frontend concerns, and it doesn't yield > a consistent UI across backends (one command that works everywhere). > >> - Could this UI also support spell-checking, via ispell or flyspell? It >> seems like "fix the spelling of a typo'd word" is something that would >> very naturally fit this whole idea. > > Sure. My implicit point here is that ispell/flyspell aren't flymake-based, so something tied to flymake may make this harder. (But maybe flymake-based spellcheckers are better anyway? I don't actually use flyspell or any other on-the-fly spellchecker, so I can't really say) >> - Could we implement all this outside of flymake, I wonder, with some >> kind of refactor-backend-functions buffer-local? > > Dunno, probably, somehow. But there's no refactor.el in core, and, > however we do it, it'll need to interact with Flymake diagnostics that > come from Flymake backends, so flymake.el seems like a natural choice. > Crucially, in flymake.el we can extend flymake-make-diagnostic to make > it as easy as possible for backends to provide fixes. Well, sure, flymake is important. But if we did it with some kind of buffer-local list of backend functions, it could also have backends which aren't flymake. IDK if there are such backends though.