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#61726: [PATCH] Eglot: Support positionEncoding capability Date: Fri, 24 Feb 2023 10:20:36 +0000 Message-ID: References: <87a614g628.fsf@gmail.com> <83cz60r7hu.fsf@gnu.org> <875ybsfvtj.fsf@gmail.com> <831qmgr17p.fsf@gnu.org> <87wn48ecdz.fsf@gmail.com> <83v8jspgnr.fsf@gnu.org> <87lekodxja.fsf@gmail.com> <83a614p4sh.fsf@gnu.org> <87cz60dus9.fsf@gmail.com> <835ybrpnqj.fsf@gnu.org> <87y1oncz09.fsf@gmail.com> <83r0ufo3uc.fsf@gnu.org> <87356vbf0b.fsf@gmail.com> 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="19268"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 61726@debbugs.gnu.org, Eli Zaretskii To: Augusto Stoffel Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Feb 24 11:21:24 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 1pVVCc-0004lW-6J for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 24 Feb 2023 11:21:22 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pVVCL-00081K-E0; Fri, 24 Feb 2023 05:21:05 -0500 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 1pVVCJ-00080h-8v for bug-gnu-emacs@gnu.org; Fri, 24 Feb 2023 05:21:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pVVCI-0004eK-UT for bug-gnu-emacs@gnu.org; Fri, 24 Feb 2023 05:21:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pVVCI-0007ur-HL for bug-gnu-emacs@gnu.org; Fri, 24 Feb 2023 05:21:02 -0500 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: Fri, 24 Feb 2023 10:21:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 61726 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 61726-submit@debbugs.gnu.org id=B61726.167723406130397 (code B ref 61726); Fri, 24 Feb 2023 10:21:02 +0000 Original-Received: (at 61726) by debbugs.gnu.org; 24 Feb 2023 10:21:01 +0000 Original-Received: from localhost ([127.0.0.1]:35992 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pVVCG-0007uC-JF for submit@debbugs.gnu.org; Fri, 24 Feb 2023 05:21:00 -0500 Original-Received: from mail-oi1-f172.google.com ([209.85.167.172]:35479) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pVVCF-0007tz-DS for 61726@debbugs.gnu.org; Fri, 24 Feb 2023 05:20:59 -0500 Original-Received: by mail-oi1-f172.google.com with SMTP id c11so16103500oiw.2 for <61726@debbugs.gnu.org>; Fri, 24 Feb 2023 02:20:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1677234053; 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=gWI9i7DWJQsPyDSCHBM28vnDzfcKEIRdqE/GOGVDW1Y=; b=YwSSozMo2mrcOr23zsSw46pDvZ799fBluWbPhQ0B9C49ocr8EJoSLRojLqNwvqACyh DZt/pxajydRt+qjY4uyHY3kU68EGyx27Uf7ES76mV1Qf8HHf02+O2I8yQYsHroMDMkpH fs2LE3g4k7ALmfVBStHIcOrZ+IORXNW/mWsbiYLmWk7c496wke9lCIYz7kFQA2JYic7r WwPd3OUrD9OvEtT3+ofNYeaDaffp3CVbe7BfgtD6gLjUx/ViEW+VDKnlKfAs9VyhD2Db 79iGQ2UOgs9nbVbfCEtsj2W/xqC2fP7FcVjucejczSrpyCVvyHLh77FOKcp2Vv4jPkcH wJhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677234053; 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=gWI9i7DWJQsPyDSCHBM28vnDzfcKEIRdqE/GOGVDW1Y=; b=dPyh+E2TFKGjps/MDjt0WnAdu+qPSHPGWS5ewz7itsOkApxqjXCTzuteIEjidPoLFL t7UlZLTsTRbVkHJMRinhnSx5ik4n1nZGBJKrfAU/EQEINrha6eyCs4bgiEdCTEQucyL+ yLRk2j/+IaeU8U8xyeIiMRL4cvQ29ePxxvH5gl+XYhhy2Gch8wqOpPTIIVMRHDVOHqf6 0x6Fpl3wRoTM+3//58MwITEbjvhZehcLGxYe2Udt19wmiS/G1qzZmUBuvPQhh+tLprox UnHQnnJXpkcaIJU2WAK7OaBDIWBJ6aCleK61cpeSZVFk/XpT4orWXWHDAkzCkBYlR9gi 6ESw== X-Gm-Message-State: AO0yUKUyjisvvYcufAaYkzOpuhZ0xvQ38ZnD+ov35142r6Sqz7dmQEPt Xg1V5DDuZvorhj3mWZpWnOo4arD0GZc7BlelQtU= X-Google-Smtp-Source: AK7set9Ugdfrii8LXXZDZ8o3A5lu4Fezp8MW6lU1NILLsa+Crp20Ngx15O0gSszlsRzGfp+t5mPamJnhF8TQ/QWdx6w= X-Received: by 2002:a54:409a:0:b0:384:253:642d with SMTP id i26-20020a54409a000000b003840253642dmr136637oii.3.1677234053704; Fri, 24 Feb 2023 02:20:53 -0800 (PST) In-Reply-To: <87356vbf0b.fsf@gmail.com> 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:256557 Archived-At: On Fri, Feb 24, 2023 at 9:15 AM Augusto Stoffel wrote= : > > ^^^^^^^^^^^ > > This last part shouldn't be necessary: we should move by characters, > > not by columns. Why is it necessary? > > Maybe Jo=C3=A3o can clarify, but I'm pretty sure this is there to support= the > UTF-16 way of counting offsets, so this ideally should move to > eglot-move-to-lsp-abiding-column. I have to be brutally honest here: I don't like this patch. I appreciate the effort, I really do, and thank for guiding us its the motions, but there are two main things I really don't like about it, and 1 that I'm on the fence about. The first thing I don't like is likely the reason that Eli is confused here. The late binding of column-counting strategies is confusing. I wrote these functions so that each one has a separate well-defined, readable-inasmuch-as-possible, vc-region-history-traceable, performant column-counting strategy. The "lsp-abiding" naming might be off, I admit, but only since LSP started supporting more than one strategy. The second thing I don't like is also due to the late-binding idea. This is a hotspot in Eglot, some of these functions are called many many times, for each LSP server interaction depending on how many document positions are exchanged (and they can be a lot). I do remember benchmarking strategies at the time and seeing a perceptible difference. Plus, this late-binding is really useless as a server will guaranteedly _not_ change its column-counting standard during the LSP session. The third thing that I'm not crazy with but I don't mind is the necessity to support the "utf-8" strategy. If "utf-16" is mandatory, and we already support "utf-32" anyway, why should we be adding this additional complexity. But, if it can be hidden behind a new pair of functions and Eli accepts it, I'm OK with it. Finally, here's a patch that doesn't use late-binding, doesn't introduce new strategies and supports "utf-32" and "utf-16" today. As you can see, the patch is nearly trivial. diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index eea8be6d1aa..ae8afa69651 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -807,6 +807,7 @@ eglot-client-capabilities :rangeFormatting `(:dynamicRegistration :json-false) :rename `(:dynamicRegistration :json-false) :inlayHint `(:dynamicRegistration :json-false) + :general `(:positionEncodings ["utf-32" "utf-16"]) :publishDiagnostics (list :relatedInformation :json-false ;; TODO: We can support :codeDescription after ;; adding an appropriate UI to @@ -1789,6 +1790,9 @@ eglot--managed-mode (add-hook 'eldoc-documentation-functions #'eglot-signature-eldoc-fun= ction nil t) (eldoc-mode 1)) + (when (eq (eglot--server-capable :positionEncoding) "utf-16") + (eglot--setq-saving eglot-move-to-column-function #'eglot-move-to-co= lumn) + (eglot--setq-saving eglot-current-column-function #'eglot-current-column)) (cl-pushnew (current-buffer) (eglot--managed-buffers (eglot-current-server)))) (t (remove-hook 'after-change-functions 'eglot--after-change t) As I said, enhancing this patch with a new pair of "current/move-to" functions that add in utf-8 support is acceptable. Jo=C3=A3o