bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Hello maintainers,

This bug I'm about to describe originated in
https://github.com/joaotavora/eglot/issues/860.  I reproduced it with:

* GNU Emacs 29.0.50 (build 2, x86_64-pc-linux-gnu) commit 88d474308e2
* company-mode, a not-too-recent version (but most recent is the same)
* the clangd LSP server 13.0.1
* Eglot d5c7c40bc

We are reasonably sure the bug is in Emacs and is a regression from
Emacs 27.2.  The bug has been "defensively" worked around in the Eglot
in its commit f775187115.

In short, it seems that in the presence of the overlays created by
company-mode and in presence of a narrowing, the move-to-column function
does not function the way it used to.

It should certainly be possible to reproduce the problem without the
reasonably complex Eglot setup, but for the time being this is all I
have.
    
  ~/Source/Emacs/emacs/src/emacs -Q -f package-initialize -L       \
  ~/Source/Emacs/company-mode -l company -f global-company-mode -l \
   eglot.el ~/tmp/issue-860/args_out_of_range.c -f eglot -f        \
   display-line-numbers-mode -f toggle-debug-on-error

Here's the args_out_of_range.c file.

  1 // args_out_of_range.c
  2 struct Book {
  3   int id;
  4   char title[50]
  5 } book = { 1024, "C" };
  6
  7 int main(int argc, char *argv[])
  8 {
  9
 10   // Error when typing the dot to make "book."
 11   book
 12   return 0;
 13 }

When one types the dot after the "book" on line 11, company-mode
displays a two-line overlay that visually encompasses line 12 after
"book", which has the "return 0;" statement.  That line happens to
also hold a warning about incorrect syntax, one that starts at column
2.

Eglot uses 'move-to-column' to go that precise place and highlight the
warning.

In Emacs 27.2, move-to-column is unaffected by previous company-mode
overlays, even if the current line is being co-used visually by the
overlay.  It moves to the right buffer position.

In Emacs master, this isn't true.  It seems to be confounded by the
company-mode overlay and moves to eob, _beyond the narrowing_, which
eventually breaks Eglot with a backtrace such as this one:

Debugger entered--Lisp error: (args-out-of-range #<buffer args_out_of_range.c> 110 124)
  encode-coding-region(110 124 utf-16 t)

Thanks for your time,
João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 21 Mar 2022 06:54:46 +0000
> Cc: dgutov@yandex.ru
> 
> In short, it seems that in the presence of the overlays created by
> company-mode and in presence of a narrowing, the move-to-column function
> does not function the way it used to.

Yes.

> Eglot uses 'move-to-column' to go that precise place and highlight the
> warning.
> 
> In Emacs 27.2, move-to-column is unaffected by previous company-mode
> overlays, even if the current line is being co-used visually by the
> overlay.  It moves to the right buffer position.
> 
> In Emacs master, this isn't true.  It seems to be confounded by the
> company-mode overlay and moves to eob, _beyond the narrowing_, which
> eventually breaks Eglot with a backtrace such as this one:

Emacs 29 has a fix for an old bug, whereby current-column etc. didn't
take display strings and overlays into account; now they do.

Of course, it could be that this fix introduced a bug in some
not-so-simple situations, so I'd appreciate if you could show a simple
recipe to reproduce the problem without the need of firing up
company-mode and/or Eglot.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

On Mon, Mar 21, 2022 at 12:32 PM Eli Zaretskii <eliz@gnu.org> wrote:

> Emacs 29 has a fix for an old bug, whereby current-column etc. didn't
> take display strings and overlays into account; now they do.
>
> Of course, it could be that this fix introduced a bug in some
> not-so-simple situations, so I'd appreciate if you could show a simple
> recipe to reproduce the problem without the need of firing up
> company-mode and/or Eglot.
>

The only pathological problem -- that I know of -- is the "not-so-simple"
example/situation I described.  But it's pretty real and easy trigger
if you happen to use those two extensions which are both reasonably
popular.

If you are aware of the behaviour change , then I don't see the point
with coming up with a simpler repro: I'd just be showing what you
confirmed: that move-to-column now is affected by overlays such
as company's whereas before (emacs 27.2/28) it didn't.

If this is a bugfix in Emacs, then the logical conclusion is that the
problem should be fixed in company-mode.  It should somehow (tm)
find a way to create a visual popup that works in terminals for which
move-to-column behaves as before. Alternatively, some variation of
move-to-column that retains the old behaviour could be invented to
give the old behaviour.

Both these solutions would be better than the current defensive
one in Eglot, which makes Eglot LSP-abiding column-finding
less performant than it used to be.

João

[-- Attachment #2: Type: text/html, Size: 2053 bytes --]

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 21 Mar 2022 16:37:18 +0000
> Cc: 54488@debbugs.gnu.org, Dmitry Gutov <dgutov@yandex.ru>
> 
> If you are aware of the behaviour change , then I don't see the point 
> with coming up with a simpler repro: I'd just be showing what you 
> confirmed: that move-to-column now is affected by overlays such
> as company's whereas before (emacs 27.2/28) it didn't.

The questions is: does current-column, move-to-column, etc. on master
indeed report the column number and move to the place whose column
number _on_display_ is as you ask them?  If they do, then this is the
correct behavior, and features that expect something else should
adapt.  But if move-to-column moves to a column that is visually
incorrect, I'd still like a simple reproducer.  Even if one must use
company-mode and eglot, can you post a recipe that doesn't involve
their installation via package.el?  Suppose I download the relevant
*.el files to my system and manually load them as needed, can you show
a recipe that loads whatever is needed and then reproduces the issue?

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Date: Mon, 21 Mar 2022 16:37:18 +0000
>> Cc: 54488@debbugs.gnu.org, Dmitry Gutov <dgutov@yandex.ru>
>> 
>> If you are aware of the behaviour change , then I don't see the point 
>> with coming up with a simpler repro: I'd just be showing what you 
>> confirmed: that move-to-column now is affected by overlays such
>> as company's whereas before (emacs 27.2/28) it didn't.
> The questions is: does current-column, move-to-column, etc. on master
> indeed report the column number and move to the place whose column
> number _on_display_ is as you ask them?  If they do, then this is the
> correct behavior, and features that expect something else should
> adapt.

I agree, though bug-compatibility is a thing sometimes :-)

Anyway, since you seem to know what changed in Emacs, can you show a
snippet which worked in some (presumably wrong) way in 27.1 and now
works correctly in master? Maybe that holds some valuable clue..

> But if move-to-column moves to a column that is visually incorrect,
> I'd still like a simple reproducer.

Hmmm.  I tried, but couldn't

(with-current-buffer (generate-new-buffer "**test**")
  (insert "foofoofoo\nbarbarbar\nbazbazbaz")
  (forward-line -1)
  (goto-char (line-beginning-position))
  (prog1
      (let ((o (make-overlay (+ 3 (point)) (+ 6 (point)))))
        (overlay-put o 'invisible t)
        (overlay-put o 'display (propertize "BAR" 'face 'highlight))
        (goto-char (line-beginning-position))
        (let ((p (point)))
          (move-to-column 3)
          (buffer-substring-no-properties p (point))))
    (pop-to-buffer (current-buffer))))

This constructs an overlay that hides/replaces some text and eventually
returns "barbar".  If the overlay weren't there it would return "bar",
obviously.  I don't now if "barbar" is correct.  It doesn't look
especially correct, but I wouldn't know what to return here either,
since the text below is invisible and has probably been skipped over.
At any rate, it's exactly the same that 27.1 does, so it's not the
problem we're after, I think.

So maybe Dmitry has some valuable insight as to how exactly company
constructs its overlay or overlays.

> Even if one must use company-mode and eglot, can you post a recipe
> that doesn't involve their installation via package.el?  Suppose I
> download the relevant *.el files to my system and manually load them
> as needed, can you show a recipe that loads whatever is needed and
> then reproduces the issue?

Yes, I can do that.  But note you still need the clangd executable
somewhere in your system to provide the completions.  In my system
it was just:

  sudo pacman -S clang

but packages exist for most (all?) GNU/Linux distributions and OSs.

Then, assuming:

  ~/Source/Emacs/emacs is a checkout of master Emacs
  ~/Source/Emacs/company-mode is a checkout of git@github.com:company-mode/company-mode.git
  ~/Source/Emacs/eglot is a checkout of git@github.com:joaotavora/eglot.git

IMPORTANT: You need Eglot to be checked out at d5c7c40b, before the
Eglot "defensive" fix.

Then finally, this should set you up and should show you the differences
beween 27.2 and master Emacs, where the former works and the latter
errors.  Obviously in the master version you don't need these 5 loads.

$ some-version-of-emacs -Q                                         \
  -l ~/Source/Emacs/emacs/lisp/emacs-lisp/eldoc.el                 \
  -l ~/Source/Emacs/emacs/lisp/progmodes/xref.el                   \
  -l ~/Source/Emacs/emacs/lisp/progmodes/flymake.el                \
  -l ~/Source/Emacs/emacs/lisp/progmodes/project.el                \
  -l ~/Source/Emacs/emacs/lisp/jsonrpc.el                          \
  -L ~/Source/Emacs/company-mode -l company -f global-company-mode \
  -L ~/Source/Emacs/eglot -l eglot.el                              \
  ~/tmp/issue-860/args_out_of_range.c                              \
  -f eglot                                                         \
  -f display-line-numbers-mode                                     \
  -f toggle-debug-on-error                                         

The args_out_of_range.c file's contents are

------BEGIN
// args_out_of_range.c
struct Book {
  int id;
  char title[50]
} book = { 1024, "C" };

int main(int argc, char *argv[])
{

  // Master errors when typing "book." i.e.
  // typing the dot after book
  book
  return 0;
}
------END


Hope this helps, 
João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Dmitry Gutov]

On 21.03.2022 23:59, João Távora wrote:
> So maybe Dmitry has some valuable insight as to how exactly company
> constructs its overlay or overlays.

You can try putting a multiline string on the 'display property.

Instead of just "BAR".

And/or use 'before-string instead of 'display.

Apart from that, I can only suggest reading the code and experimenting.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 21.03.2022 23:59, João Távora wrote:
>> So maybe Dmitry has some valuable insight as to how exactly company
>> constructs its overlay or overlays.
>
> You can try putting a multiline string on the 'display property.
>
> Instead of just "BAR".

I tried that, but I don't know how to align it like company does.

> Apart from that, I can only suggest reading the code and
> experimenting.

I admit I was hoping that you as company maintainer/specialist :-) could
provide a small snippet to create a dummy but company-realistic overlay
of, say, two completion "foo" and "bar" in the middle of any buffer.  Or
perhaps explain in layman's terms how company constructs its overlays.

Not asking that you get to the root of the move-to-column problem, which
is clearly a regression (presumably a regression to correct behaviour,
but a regression nonetheless) just help perhaps come up with a simpler
repro to assist Eli in seeing the issue.

But there's no urgency.  This, I understand, is not in Emacs 28 and
there is a workaround in Eglot.

João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  54488@debbugs.gnu.org
> Date: Tue, 22 Mar 2022 09:48:06 +0000
> 
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
> > On 21.03.2022 23:59, João Távora wrote:
> >> So maybe Dmitry has some valuable insight as to how exactly company
> >> constructs its overlay or overlays.
> >
> > You can try putting a multiline string on the 'display property.
> >
> > Instead of just "BAR".
> 
> I tried that, but I don't know how to align it like company does.
> 
> > Apart from that, I can only suggest reading the code and
> > experimenting.
> 
> I admit I was hoping that you as company maintainer/specialist :-) could
> provide a small snippet to create a dummy but company-realistic overlay
> of, say, two completion "foo" and "bar" in the middle of any buffer.  Or
> perhaps explain in layman's terms how company constructs its overlays.
> 
> Not asking that you get to the root of the move-to-column problem, which
> is clearly a regression (presumably a regression to correct behaviour,
> but a regression nonetheless) just help perhaps come up with a simpler
> repro to assist Eli in seeing the issue.

Thank you for your efforts.  I will take a look at this issue when I
have time, hopefully soon enough.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Dmitry Gutov]

On 22.03.2022 11:48, João Távora wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 21.03.2022 23:59, João Távora wrote:
>>> So maybe Dmitry has some valuable insight as to how exactly company
>>> constructs its overlay or overlays.
>>
>> You can try putting a multiline string on the 'display property.
>>
>> Instead of just "BAR".
> 
> I tried that, but I don't know how to align it like company does.

Have you tried 'before-string as well?

>> Apart from that, I can only suggest reading the code and
>> experimenting.
> 
> I admit I was hoping that you as company maintainer/specialist :-) could
> provide a small snippet to create a dummy but company-realistic overlay
> of, say, two completion "foo" and "bar" in the middle of any buffer.  Or
> perhaps explain in layman's terms how company constructs its overlays.

I think it would be faster for you to just read the code.

I'm not sure you need a realistic-looking overlay, though.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

On Tue, Mar 22, 2022 at 12:33 PM Dmitry Gutov <dgutov@yandex.ru> wrote:


> > I tried that, but I don't know how to align it like company does.
> Have you tried 'before-string as well?
>

Yes, same thing.

I think it would be faster for you to just read the code.
>

I think you overestimate my ability.


> I'm not sure you need a realistic-looking overlay, though.
>

It's a good idea to drift as little as possible from the only case that we
know that actually triggers the problem, which is "real" company mode.

João

[-- Attachment #2: Type: text/html, Size: 1261 bytes --]

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> Date: Tue, 22 Mar 2022 14:16:29 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 54488@debbugs.gnu.org, dgutov@yandex.ru
> 
> > > You can try putting a multiline string on the 'display property.
> > >
> > > Instead of just "BAR".
> > 
> > I tried that, but I don't know how to align it like company does.
> > 
> > > Apart from that, I can only suggest reading the code and
> > > experimenting.
> > 
> > I admit I was hoping that you as company maintainer/specialist :-) could
> > provide a small snippet to create a dummy but company-realistic overlay
> > of, say, two completion "foo" and "bar" in the middle of any buffer.  Or
> > perhaps explain in layman's terms how company constructs its overlays.
> > 
> > Not asking that you get to the root of the move-to-column problem, which
> > is clearly a regression (presumably a regression to correct behaviour,
> > but a regression nonetheless) just help perhaps come up with a simpler
> > repro to assist Eli in seeing the issue.
> 
> Thank you for your efforts.  I will take a look at this issue when I
> have time, hopefully soon enough.

I think I see the problem.  Stay tuned.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> Date: Tue, 22 Mar 2022 15:57:17 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 54488@debbugs.gnu.org, dgutov@yandex.ru
> 
> I think I see the problem.  Stay tuned.

I definitely see a problem with display strings with embedded
newlines, but I don't think I understand how it affects your use case.

Try this simple recipe:

  emacs -Q
  M-: (overlay-put (make-overlay 91 100) 'display "FOOBAR\nBAZQUUX") RET

You should then see this in *scratch (I indented by 2 columns, so it
stands out):

  ;; This buffer is for text that is not saved, and for Lisp evaluation.
  ;; To create a fileFOOBAR
  BAZQUUXt it with C-x C-f and enter text in its buffer.

(with the "FOOBAR\nBAZQUUX" part intangible, i.e. you cannot put the
cursor inside).

Now go to the 't' at the end of "BAZQUUXt" and type "C-x =".  In Emacs
28 and earlier you will see "column=28", in Emacs 29 you will see
"column=32".  This is because Emacs 28 ignores the display string and
examines only the buffer text, which is "covered" by the display
property.  By contrast, Emacs 29 ignores the "covered" buffer text
(since it's invisible on display) and examines only the characters of
the display string.  Since the newline has a width of zero, we get 9
columns between "file" and "t with" in Emacs 28, but 13 in Emacs 29.

So yes, Emacs 29 behaves differently, but the important part is that
both Emacs 28 and Emacs 29 are wrong: the _real_ column at 't' is 7
(columns are zero-based).  IOW, both Emacs 28 and Emacs 29 fail to
account for the fact that the newline inside the display string resets
the column to zero.

I could perhaps come up with a solution for that, and have the above
snippet correctly return 7 as the column.  But I don't see how this
will help company-mode and/or Eglot, since the value returned by Emacs
28 makes no more sense than the value returned by Emacs 29.

So I need your help with understanding what exactly fails in the
original use case, due to this change.  Specifically, how does the
code involved in this use move-to-column/current-column when there are
display strings with embedded newlines around?

TIA

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

> So I need your help with understanding what exactly fails in the
> original use case, due to this change.  Specifically, how does the
> code involved in this use move-to-column/current-column when there are
> display strings with embedded newlines around?

I think I alluded to/explained this in the original bug report.  I'll
try again: eglot needs just move-to-column that it runs in a narrowed
region containing only the line and obtain an upper bound of the
"LSP-abiding" column that the LSP server means (unfortunately the
LSP-abiding column may differ from Emacs's understanding of columns
because of different assumpting regarding length of character
encodings).

Eglot then gets (point) and tries "encode-coding-string" to measure
things.

In Emacs 29, before the fix, that (point) seems to -- very surprisingly
-- return something beyond the narrowed region and so
encode-coding-string will complain with args out of range.

In the example I gave you, that narrowed region, (point-max) is 176.

- that (point) form returns 179 in Emacs 29 -- error obviously.
- that (point) form returns 167 in Emacs 27 -- no error.

This is the code more or less in Eglot:

(defun eglot-move-to-lsp-abiding-column (column)
  "Move to COLUMN abiding by the LSP spec."
  (save-restriction
    (cl-loop
     with lbp = (line-beginning-position)
     initially
     (narrow-to-region lbp (line-end-position))
     (move-to-column column)
     for diff = (- column
                   (/ (- (length (encode-coding-region (or lbp (line-beginning-position))
                                      (point) 'utf-16 t))
                                  2)
                       2))
     until (zerop diff)
     do (condition-case eob-err
            (forward-char (/ (if (> diff 0) (1+ diff) (1- diff)) 2))
          (end-of-buffer (cl-return eob-err))))))

João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: dgutov@yandex.ru,  54488@debbugs.gnu.org
> Date: Tue, 22 Mar 2022 14:54:13 +0000
> 
> In Emacs 29, before the fix, that (point) seems to -- very surprisingly
> -- return something beyond the narrowed region and so
> encode-coding-string will complain with args out of range.

I don't understand how (point) can return a value outside of the
narrowed region, unless it runs with the narrowing temporarily
disabled.  Can you show me the code with that call to 'point' again?
I don't think I see it in your OP.

> (defun eglot-move-to-lsp-abiding-column (column)
>   "Move to COLUMN abiding by the LSP spec."
>   (save-restriction
>     (cl-loop
>      with lbp = (line-beginning-position)
>      initially
>      (narrow-to-region lbp (line-end-position))
>      (move-to-column column)
>      for diff = (- column
>                    (/ (- (length (encode-coding-region (or lbp (line-beginning-position))
>                                       (point) 'utf-16 t))
>                                   2)
>                        2))
>      until (zerop diff)
>      do (condition-case eob-err
>             (forward-char (/ (if (> diff 0) (1+ diff) (1- diff)) 2))
>           (end-of-buffer (cl-return eob-err))))))

Why do you have to use move-to-column instead of forward-char?

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1883 bytes --]

On Tue, Mar 22, 2022 at 3:22 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Cc: dgutov@yandex.ru,  54488@debbugs.gnu.org
> > Date: Tue, 22 Mar 2022 14:54:13 +0000
> >
> > In Emacs 29, before the fix, that (point) seems to -- very surprisingly
> > -- return something beyond the narrowed region and so
> > encode-coding-string will complain with args out of range.
>
> I don't understand how (point) can return a value outside of the
> narrowed region, unless it runs with the narrowing temporarily
> disabled.  Can you show me the code with that call to 'point' again?
> I don't think I see it in your OP.
>

It's the snippet below. but the indentation probably botched it.
I inlined a function in that snippet so you can see the move-to-column
and the (point) in the same function (normally they live in different
functions).

> (defun eglot-move-to-lsp-abiding-column (column)
> >   "Move to COLUMN abiding by the LSP spec."
> >   (save-restriction
> >     (cl-loop
> >      with lbp = (line-beginning-position)
> >      initially
> >      (narrow-to-region lbp (line-end-position))
> >      (move-to-column column)
> >      for diff = (- column
> >                    (/ (- (length (encode-coding-region (or lbp
> (line-beginning-position))
> >                                         (point) 'utf-16 t))   ;; <=====
> THAT (point)
> >                                   2)
> >                        2))
> >      until (zerop diff)
> >      do (condition-case eob-err
> >             (forward-char (/ (if (> diff 0) (1+ diff) (1- diff)) 2))
> >           (end-of-buffer (cl-return eob-err))))))
>
> Why do you have to use move-to-column instead of forward-char?
>

I don't remember! Is it exactly equivalent if you're standing in the
beginning of the line?

João


-- 
João Távora

[-- Attachment #2: Type: text/html, Size: 2907 bytes --]

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Tue, 22 Mar 2022 16:06:48 +0000
> Cc: Dmitry Gutov <dgutov@yandex.ru>, 54488@debbugs.gnu.org
> 
>  I don't understand how (point) can return a value outside of the
>  narrowed region, unless it runs with the narrowing temporarily
>  disabled.  Can you show me the code with that call to 'point' again?
>  I don't think I see it in your OP.
> 
> It's the snippet below. but the indentation probably botched it.
> I inlined a function in that snippet so you can see the move-to-column
> and the (point) in the same function (normally they live in different 
> functions).
> 
>  > (defun eglot-move-to-lsp-abiding-column (column)
>  >   "Move to COLUMN abiding by the LSP spec."
>  >   (save-restriction
>  >     (cl-loop
>  >      with lbp = (line-beginning-position)
>  >      initially
>  >      (narrow-to-region lbp (line-end-position))
>  >      (move-to-column column)
>  >      for diff = (- column
>  >                    (/ (- (length (encode-coding-region (or lbp (line-beginning-position))
>  >                                         (point) 'utf-16 t))   ;; <===== THAT (point)
>  >                                   2)
>  >                        2))
>  >      until (zerop diff)
>  >      do (condition-case eob-err
>  >             (forward-char (/ (if (> diff 0) (1+ diff) (1- diff)) 2))
>  >           (end-of-buffer (cl-return eob-err))))))

I don't see how this could cause the problem you describe, but please
note that encode-coding-region generally changes the text in the
region, so maybe what you consider to be outside the restriction
isn't?

Anyway, do you have an example of text in which this function causes
point to return such problematic values?

>  Why do you have to use move-to-column instead of forward-char?
> 
> I don't remember! Is it exactly equivalent if you're standing in the beginning of the line? 

Yes, if you have only text in the buffer (no images etc.).

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

On Tue, Mar 22, 2022 at 4:54 PM Eli Zaretskii <eliz@gnu.org> wrote:

>  >      do (condition-case eob-err
> >  >             (forward-char (/ (if (> diff 0) (1+ diff) (1- diff)) 2))
> >  >           (end-of-buffer (cl-return eob-err))))))
>
> I don't see how this could cause the problem you describe, but please
> note that encode-coding-region generally changes the text in the
> region, so maybe what you consider to be outside the restriction
> isn't?
>

I passed it t as the last argument, so it should be non-destructive to the
buffer.

Anyway, do you have an example of text in which this function causes
> point to return such problematic values?
>

The only example I have is the one I described already, as best as I could.
A user reported it to me, i installed clangd, and I reproduced it very
easily.

If you could consider installing clangd then running that ready-to-use
recipe, I'd venture to say it's the easiest way for you to understand the
problem.

>  Why do you have to use move-to-column instead of forward-char?
> >
> > I don't remember! Is it exactly equivalent if you're standing in the
> beginning of the line?
>
> Yes, if you have only text in the buffer (no images etc.).
>

Hmmm, it's interesting. Maybe I should just switch to that. Seems to fix
it. Until someone adds images to the source code i suppose. What's your
"etc"?

João

>

[-- Attachment #2: Type: text/html, Size: 2594 bytes --]

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Dmitry Gutov]

On 22.03.2022 23:05, João Távora wrote:
>      >  Why do you have to use move-to-column instead of forward-char?
>      >
>      > I don't remember! Is it exactly equivalent if you're standing in
>     the beginning of the line?
> 
>     Yes, if you have only text in the buffer (no images etc.).
> 
> 
> Hmmm, it's interesting. Maybe I should just switch to that. Seems to fix 
> it. Until someone adds images to the source code i suppose. What's your 
> "etc"?

Tabs?

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Tue, Mar 22, 2022 at 11:55 PM Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 22.03.2022 23:05, João Távora wrote:
> >      >  Why do you have to use move-to-column instead of forward-char?
> >      >
> >      > I don't remember! Is it exactly equivalent if you're standing in
> >     the beginning of the line?
> >
> >     Yes, if you have only text in the buffer (no images etc.).
> >
> >
> > Hmmm, it's interesting. Maybe I should just switch to that. Seems to fix
> > it. Until someone adds images to the source code i suppose. What's your
> > "etc"?
>
> Tabs?
>

Aha! yup.  This idea won't fly.  Thanks, Dmitry, I was about to push that
happily, but people definitely use tabs in their source code.

João

[-- Attachment #2: Type: text/html, Size: 1276 bytes --]

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Tue, 22 Mar 2022 21:05:09 +0000
> Cc: Dmitry Gutov <dgutov@yandex.ru>, 54488@debbugs.gnu.org
> 
>  >  >      do (condition-case eob-err
>  >  >             (forward-char (/ (if (> diff 0) (1+ diff) (1- diff)) 2))
>  >  >           (end-of-buffer (cl-return eob-err))))))
> 
>  I don't see how this could cause the problem you describe, but please
>  note that encode-coding-region generally changes the text in the
>  region, so maybe what you consider to be outside the restriction
>  isn't?
> 
> I passed it t as the last argument, so it should be non-destructive to the buffer.

But you do that in a loop AFAIU, so one iteration could affect the
next ones.  But I'm just hand-waving here.

>  Anyway, do you have an example of text in which this function causes
>  point to return such problematic values?
> 
> The only example I have is the one I described already, as best as I could. A user reported it to me, i installed
> clangd, and I reproduced it very easily.
> 
> If you could consider installing clangd then running that ready-to-use recipe, I'd venture to say it's the easiest
> way for you to understand the problem.

Sorry, not going to happen.  And I don't see why that would be
necessary: the problem happens entirely in Emacs Lisp, so the only
thing we need from clangd is its output that Emacs uses.  Can't you or
someone collect that and include it in the recipe?

>  >  Why do you have to use move-to-column instead of forward-char?
>  > 
>  > I don't remember! Is it exactly equivalent if you're standing in the beginning of the line? 
> 
>  Yes, if you have only text in the buffer (no images etc.).
> 
> Hmmm, it's interesting. Maybe I should just switch to that. Seems to fix it. Until someone adds images to the
> source code i suppose. What's your "etc"?

Any "display element" that is not text, like xwidgets.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> Date: Wed, 23 Mar 2022 01:55:17 +0200
> Cc: 54488@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 22.03.2022 23:05, João Távora wrote:
> >      >  Why do you have to use move-to-column instead of forward-char?
> >      >
> >      > I don't remember! Is it exactly equivalent if you're standing in
> >     the beginning of the line?
> > 
> >     Yes, if you have only text in the buffer (no images etc.).
> > 
> > 
> > Hmmm, it's interesting. Maybe I should just switch to that. Seems to fix 
> > it. Until someone adds images to the source code i suppose. What's your 
> > "etc"?
> 
> Tabs?

Tabs are a problem anyway, because the compiler cannot know how wide
the tab is in the Emacs buffer.  So if move-to-column is used to get
to the place where the error is indicated, tabs should be a problem
that is already handled specially, I think.  And if so, it could be
handled specially (in a different way) when forward-char is used
instead.

Or what am I missing?

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Wed, 23 Mar 2022 01:11:21 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, 54488@debbugs.gnu.org
> 
>  Tabs?
> 
> Aha! yup.  This idea won't fly.  Thanks, Dmitry, I was about to push that 
> happily, but people definitely use tabs in their source code.  

How do you know to which column to move when tabs are involved?  If
clangd assumes a tab is at a multiple of 8, and the user has tab-width
customized to a different value, you will end up in a wrong place
anyway.  So this case should already cause problems, and if not,
there's some code somewhere that handles it; let it handle it now with
forward-char as well.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Date: Tue, 22 Mar 2022 21:05:09 +0000
>> Cc: Dmitry Gutov <dgutov@yandex.ru>, 54488@debbugs.gnu.org
>> 
>>  >  >      do (condition-case eob-err
>>  >  >             (forward-char (/ (if (> diff 0) (1+ diff) (1- diff)) 2))
>>  >  >           (end-of-buffer (cl-return eob-err))))))
>> 
>>  I don't see how this could cause the problem you describe, but please
>>  note that encode-coding-region generally changes the text in the
>>  region, so maybe what you consider to be outside the restriction
>>  isn't?
>> 
>> I passed it t as the last argument, so it should be non-destructive to the buffer.
>
> But you do that in a loop AFAIU, so one iteration could affect the
> next ones.  But I'm just hand-waving here.

I'm not doing it a loop for destructive effect.  I'm doing it for
measuring.  If you're interested in the full 2018 story, it's
https://github.com/joaotavora/eglot/pull/125. 
 
>>  Anyway, do you have an example of text in which this function causes
>>  point to return such problematic values?
>> 
>> The only example I have is the one I described already, as best as I could. A user reported it to me, i installed
>> clangd, and I reproduced it very easily.
>> 
>> If you could consider installing clangd then running that ready-to-use recipe, I'd venture to say it's the easiest
>> way for you to understand the problem.
>
> Sorry, not going to happen.

Is it because it's a non GPL server, or just because you don't like to
install LSP servers?  Other LSP servers will probably have the same
problem.

> And I don't see why that would be necessary: the problem happens
> entirely in Emacs Lisp, so the only thing we need from clangd is its
> output that Emacs uses.  Can't you or someone collect that and include
> it in the recipe?

Actually, I do have that, and a LSP could be built that simply replays
those logs. But this is too complex.

I guess if you could point me to (your?) commit that changed the
behaviour in I can do the reproduction here and see the problem
myself, when I have time.

João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Date: Wed, 23 Mar 2022 01:11:21 +0000
>> Cc: Eli Zaretskii <eliz@gnu.org>, 54488@debbugs.gnu.org
>> 
>>  Tabs?
>> 
>> Aha! yup.  This idea won't fly.  Thanks, Dmitry, I was about to push that 
>> happily, but people definitely use tabs in their source code.  
>
> How do you know to which column to move when tabs are involved?  If
> clangd assumes a tab is at a multiple of 8, and the user has tab-width
> customized to a different value, you will end up in a wrong place
> anyway.  So this case should already cause problems, and if not,
> there's some code somewhere that handles it; let it handle it now with
> forward-char as well.

As I explained before, move-to-column is only used to obtain an upper
bound of a (binary) search backwards.  That search is for the
"LSP-abiding" column, which is tricky and based on UTF-16 code units
even though the file is usually UTF-8 encoded.  The gory details are all
in this issue https://github.com/joaotavora/eglot/pull/125 from 2018.

So if I start using forward-char, my upper bound will be unnecessarily
conservative.  The whole thing won't be wrong but slower.  And because
there can be very many diagnostics and things being annotated in
positions, it can make a difference.  I benchmarked it at the time.

I also think you're supposed to agree with the LSP server about
tab-width and stuff.  I just keep my tabs at 8 like They intended and I
don't have that problem, because at least clangd correctly assumes that
default.

João

PS: I do invite you to read that old Eglot issue.  Since you're an
expert on coding system conversion, maybe you know of a better, faster
way to find the correct LSPish column in an Emacs buffer.  Maybe the
whole search idea is completely overwrought.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

On Wed, Mar 23, 2022 at 10:09 AM João Távora <joaotavora@gmail.com> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> PS: I do invite you to read that old Eglot issue.  Since you're an
> expert on coding system conversion, maybe you know of a better, faster
> way to find the correct LSPish column in an Emacs buffer.  Maybe the
> whole search idea is completely overwrought.
>

These issues may give additional context about the need for this particular
move-to-column dance.

https://github.com/joaotavora/eglot/issues/125 (the one I gave you already)
https://github.com/joaotavora/eglot/issues/124 (the bug that prompted the
125 fix)
https://github.com/joaotavora/eglot/issues/361 (an easier to grasp
manifestation of the problem)

Also, I think the current fix of Eglot is decent.  It will be slower when
there are
company-mode overlays, but those are in much lower numbers compared to
LSP-abiding
positioning requests. And some seem to be moving away from company-mode
and its overlay-based completion display anyway.  So this is not urgent.

But I still do think there was a regression in Emacs somewhere: I've
described an
unequivocal reproduction recipe, just not something that can be shared
among us,
due to technical (or licensing) hurdles.

João

[-- Attachment #2: Type: text/html, Size: 2032 bytes --]

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: dgutov@yandex.ru,  54488@debbugs.gnu.org
> Date: Wed, 23 Mar 2022 10:04:40 +0000
> 
> >> If you could consider installing clangd then running that ready-to-use recipe, I'd venture to say it's the easiest
> >> way for you to understand the problem.
> >
> > Sorry, not going to happen.
> 
> Is it because it's a non GPL server, or just because you don't like to
> install LSP servers?  Other LSP servers will probably have the same
> problem.

It's because I cannot afford the time and efforts of setting up
another compiler suite on my machine.  Too many things on my plate
already, sorry.

> I guess if you could point me to (your?) commit that changed the
> behaviour in I can do the reproduction here and see the problem
> myself, when I have time.

The commit is here: 4243747.  It fixed bug#53795.  Before the change
the low-level routines that move-to-column and current-column called
ignored display strings shown in the buffer; now they take the display
strings into consideration.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Wed, 23 Mar 2022 11:08:55 +0000
> Cc: Dmitry Gutov <dgutov@yandex.ru>, 54488@debbugs.gnu.org
> 
>  PS: I do invite you to read that old Eglot issue.  Since you're an
>  expert on coding system conversion, maybe you know of a better, faster
>  way to find the correct LSPish column in an Emacs buffer.  Maybe the
>  whole search idea is completely overwrought.
> 
> These issues may give additional context about the need for this particular
> move-to-column dance.
> 
> https://github.com/joaotavora/eglot/issues/125 (the one I gave you already) 
> https://github.com/joaotavora/eglot/issues/124 (the bug that prompted the 125 fix)
> https://github.com/joaotavora/eglot/issues/361 (an easier to grasp manifestation of the problem) 

I see that you had problems reconciling the LSP idea of "columns" with
that of Emacs.  If LSP indeed works in UTF-16 (I don't know, but I
have no reason to doubt that), then I think your solution is decent,
although actually encoding stuff could be overhead: after all, whether
a given codepoint takes 1 or 2 UTF-16 code units can be easily
established by looking at the codepoints themselves.  But that's an
optimization.

> But I still do think there was a regression in Emacs somewhere: I've described an
> unequivocal reproduction recipe, just not something that can be shared among us, 
> due to technical (or licensing) hurdles.

It is this single issue that puzzles me.  AFAIU, you are saying that
in some situation, calling move-to-column causes point to be set
outside of the current accessible region of the buffer, which I cannot
understand how it can happen.  Everything I tried yields the same
correct result: move-to-column always stops at the end of the
accessible portion of the buffer, no matter what ridiculously large
argument I pass to it.  And the change which we think caused this
problem just makes the column values for some situation smaller or
larger, that's all, so it "just" affects the argument to
move-to-column.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

> It's because I cannot afford the time and efforts of setting up
> another compiler suite on my machine.  Too many things on my plate
> already, sorry.

That's OK.  I can certainly relate :-) Though in many platforms I know
setting up this clangd language server is just about installing the
"clang" package available in many distribution.

On a tangent, it'd be great if Emacs could come with at least one
ready-to-use LSP server for debugging things like these.  Maybe an Elisp
server, although LSP servers for Elisp are kind of silly IMO, since LSP
is very inferior to Emacs's own facilities for debugging Elisp.  So a C
LSP server would be the next natural choice.

>> I guess if you could point me to (your?) commit that changed the
>> behaviour in I can do the reproduction here and see the problem
>> myself, when I have time.
> The commit is here: 4243747.  It fixed bug#53795.  Before the change
> the low-level routines that move-to-column and current-column called
> ignored display strings shown in the buffer; now they take the display
> strings into consideration.

OK thanks, I'll have a look when there's a bit more space on my plate.

João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> https://github.com/joaotavora/eglot/issues/125 (the one I gave you already) 
>> https://github.com/joaotavora/eglot/issues/124 (the bug that prompted the 125 fix)
>> https://github.com/joaotavora/eglot/issues/361 (an easier to grasp manifestation of the problem) 
>
> I see that you had problems reconciling the LSP idea of "columns" with
> that of Emacs.  If LSP indeed works in UTF-16 (I don't know, but I
> have no reason to doubt that), then I think your solution is decent,
> although actually encoding stuff could be overhead: after all, whether
> a given codepoint takes 1 or 2 UTF-16 code units can be easily
> established by looking at the codepoints themselves.  But that's an
> optimization.

An interesting one, though.  I think I follow.  Is there some easy way
in Emacs to measure how many UTF-16 code units a given codepoint takes?
I'm afraid I evicted this knowledge from the cache since I made that fix
2,5 years ago... Is a "codepoint" here a character in a string/buffer?

João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: dgutov@yandex.ru,  54488@debbugs.gnu.org
> Date: Thu, 24 Mar 2022 15:01:30 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I see that you had problems reconciling the LSP idea of "columns" with
> > that of Emacs.  If LSP indeed works in UTF-16 (I don't know, but I
> > have no reason to doubt that), then I think your solution is decent,
> > although actually encoding stuff could be overhead: after all, whether
> > a given codepoint takes 1 or 2 UTF-16 code units can be easily
> > established by looking at the codepoints themselves.  But that's an
> > optimization.
> 
> An interesting one, though.  I think I follow.  Is there some easy way
> in Emacs to measure how many UTF-16 code units a given codepoint takes?

That's easy: any codepoint above #xFFFF needs 2 UTF-16 code units.

> I'm afraid I evicted this knowledge from the cache since I made that fix
> 2,5 years ago... Is a "codepoint" here a character in a string/buffer?

Yes.

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> An interesting one, though.  I think I follow.  Is there some easy way
>> in Emacs to measure how many UTF-16 code units a given codepoint takes?
>
> That's easy: any codepoint above #xFFFF needs 2 UTF-16 code units.

>> I'm afraid I evicted this knowledge from the cache since I made that fix
>> 2,5 years ago... Is a "codepoint" here a character in a string/buffer?
>
> Yes.

Indeed seems easy enough :-).  So

(defun measure (s) (cl-loop for c across s sum (if (> c #xFFFF) 2 1)))

?

João

bug#54488: 29.0.50; move-to-column/overlay-related regression in latest master, perhaps 28?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: dgutov@yandex.ru,  54488@debbugs.gnu.org
> Date: Thu, 24 Mar 2022 16:03:59 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> An interesting one, though.  I think I follow.  Is there some easy way
> >> in Emacs to measure how many UTF-16 code units a given codepoint takes?
> >
> > That's easy: any codepoint above #xFFFF needs 2 UTF-16 code units.
> 
> >> I'm afraid I evicted this knowledge from the cache since I made that fix
> >> 2,5 years ago... Is a "codepoint" here a character in a string/buffer?
> >
> > Yes.
> 
> Indeed seems easy enough :-).  So
> 
> (defun measure (s) (cl-loop for c across s sum (if (> c #xFFFF) 2 1)))
> 
> ?

Yes, I think so.