[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#61726: [PATCH] Eglot: Support positionEncoding capability
From: |
João Távora |
Subject: |
bug#61726: [PATCH] Eglot: Support positionEncoding capability |
Date: |
Fri, 24 Feb 2023 11:43:18 +0000 |
On Fri, Feb 24, 2023 at 11:27 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Augusto Stoffel <arstoffel@gmail.com>
> > Cc: joaotavora@gmail.com, 61726@debbugs.gnu.org
> > Date: Fri, 24 Feb 2023 10:15:48 +0100
> >
> > >> -(defun eglot-current-column () (- (point) (line-beginning-position)))
> > >> +(defun eglot-current-column ()
> > >> + "Calculate current column, counting Unicode codepoints."
> > >> + (- (point) (line-beginning-position)))
> > >
> > > Can we please take this opportunity to get rid of the confusing
> > > "column" terminology? As became evident from this discussion, we are
> > > not talking columns here, we are talking offsets in characters from
> > > BOL. So something like "pos" or "linepos" or "line-offset" should be
> > > better.
> > >
> > > João, are you okay with such a sweeping change in all of eglot.el?
> >
> > I like linepos, if João is fine with not making the absolute minimal
> > amount of changes to the code.
>
> João?
eglot-current-column is a user-visible function. would need
obsoletion aliases. Are you sure it isn't better just to add some
clarifying comments? I fear for my ability to recall details about this
code with such a sweeping rename, accuracy-improving as it may be.
So it's a balancing act, your call.
>
> > >> + (tab-width 1)
> > > ^^^^^^^^^^^
> > > This last part shouldn't be necessary: we should move by characters,
> > > not by columns. Why is it necessary?
> >
> > Maybe João 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.
>
> Then perhaps the UTF-16 way of counting offsets should be changed as
> well.
I've vc-region-history'ed it to:
commit 2cf7905887f2137869f44c3383a55636e38b4b81
Author: Michal Krzywkowski <k.michal@zoho.com>
Date: Mon Nov 19 21:22:14 2018 +0100
Treat tab characters as 1 column wide in position conversion functions
Fixes https://github.com/joaotavora/eglot/issues/158.
* eglot.el (eglot--pos-to-lsp-position): Call
eglot-current-column-function with tab-width bound to 1.
(eglot--lsp-position-to-point): Call eglot-move-to-column-function
with tab-width bound to 1.
Following the link, I read this there:
"This is because move-to-column and current-column count each tab
character as
tab-width chars."
And, as far as I remember, at the time we were indeed using move-to-column and
current-column. But now we aren't anymore.
So maybe, just maybe, this can be removed. And the full test suite must
run afterwards. And then probably more tests should be added.
João
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, (continued)
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Augusto Stoffel, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, João Távora, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Augusto Stoffel, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, João Távora, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Augusto Stoffel, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, João Távora, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Augusto Stoffel, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Eli Zaretskii, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, João Távora, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Eli Zaretskii, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability,
João Távora <=
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Eli Zaretskii, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, João Távora, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Eli Zaretskii, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Augusto Stoffel, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Augusto Stoffel, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Eli Zaretskii, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Augusto Stoffel, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, João Távora, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, Eli Zaretskii, 2023/02/24
- bug#61726: [PATCH] Eglot: Support positionEncoding capability, João Távora, 2023/02/24