Applying TextEdits fails when line contains tabs

[mkcms]

The recent commits that fixed issue #124 introduced a bug which is triggered when tab characters are present in buffer.

In this file

// -*- indent-tabs-mode: t; c-file-style: "linux"; -*-

int main()
{
	return 0
}

When I execute the code action for the error (missing ;), I end up with:

// -*- indent-tabs-mode: t; c-file-style: "linux"; -*-

int main()
{
	r;eturn 0
}

But in this file, executing the code action is correct:

// -*- indent-tabs-mode: t; c-file-style: "linux"; -*-

int main()
{
	return 0
}

// Local Variables:
// eglot-move-to-column-function: forward-char
// eglot-current-column-function: (lambda nil (- (point) (point-at-bol)))
// End:

This is because move-to-column and current-column count each tab character as tab-width chars.

[joaotavora]

Does it mean that for the LSP server a TAB character counts as one column? Always and per the spec?
If so:

1. can you check if it works by let-binding tab-width to 1 around our calls to eglot-move-to-column-function and eglot-current-column-function?
2. If 1 doesn't work, I think your suggestions are good default values for the two variables.

[cmm]

@joaotavora I was just starting to wonder why identifier highlighting got all weird lately, and it's the same bug (and your suggestion 1 does indeed fix it)

[mkcms]

@joaotavora @cmm

1. can you check if it works by let-binding `tab-width` to 1 around our calls to eglot-move-to-column-function and `eglot-current-column-function`?

Yes, that works, but it doesn't work if you insert a control character such as ^L (C-q C-l) at the beginning of line. These control characters are also counted as more than 1 character.

[joaotavora]

That's a page break, it usually comes on a line of its own. If it doesn't it's kinda lame. I say "wontfix" for that.

[joaotavora]

@mkcms unless you can find more crazy cases go ahead and commit the fix 1 if you can.

[mkcms]

That's a page break, it usually comes on a line of its own.

You can put those characters anywhere you want in code. Sure, you should probably always escape them, but the current behavior will be annoying for users who don't or can't (because they work with some legacy or bad code bases).

[joaotavora]

You can put those characters anywhere you want in code.

I didn't say you couldn't . I said it "usually" comes in a line of its own.

Sure, you should probably always escape them, but the current behavior will be annoying for users who don't or can't (because they work with some legacy or bad code bases).

There not nearly as pervasive as TAB. Every ^L I've ever seen came on a line of its own, or at the very least at the end of a line. I won't needlessly complicate eglot for some super-remote corner case. So unless you can convince me it's not remote at all, I'm going with 1.

[mkcms]

@mkcms unless you can find more crazy cases go ahead and commit the fix 1 if you can.

Done.

I won't needlessly complicate eglot for some super-remote corner case.

I don't think that the solutions I provided in 1st post were very complicated. They wouldn't even increase the line count of eglot.el 😃

[joaotavora]

It't not about line count, it's about complexity and readability. This makes it clearer, IMO, that in LSP, a character is really a character, not a column.

[mkcms]

It't not about line count, it's about complexity and readability. This makes it clearer, IMO, that in LSP, a character is really a character, not a column.

Of course, I have end-user experience in mind, you're thinking about code readability.

[joaotavora]

Of course, I have end-user experience in mind, you're thinking about code readability.

I don't understand. Both fixes improve the end-user experience (modulo the ^L case) I just happen to believe my fix is more readable.

That said, if you think yours is more readable, and given that the way things are going you will probably become the Eglot maintainer eventually, go ahead and use your version. I agree it's not much worse (but it the forward-char one does rely on the fact that it's called from the first column, so it breaks the contract a little).

[mkcms]

Both fixes improve the end-user experience (modulo the ^L case)

Yes, sorry. I just remember the times without LSP, when I had to configure language features in Emacs manually (rtags, elpy, irony-mode...). Now I'm kind of paranoid about tiny issues like that, because I remember how it can annoy someone (although in this case the fix is trivial).

go ahead and use your version.

I will wait a bit, perhaps we can add a comment explaining why the fix is necessary and maybe add a TODO.