Remove highlight overlay immediately when symbol is edited

[astoff]

Currently, the textDocument/documentHighlight overlays stay when the highlighted symbol is edited, which is is distracting and arguably incorrect.

With this PR, the highlight disappears as soon as the symbol is edited (other highlights stick around, which feels right to me).

[joaotavora]

Seems sensible, but do we need a category? It's only really useful when there's more than one usage, right?

[astoff]

I'm not sure what the pros and cons are in this case. There's one usage, but it occurs in a loop; I though it would be a bit smarter to create a list with a lambda in it only once. Anyway, should I change that?

Also, just to make sure: if we delete the overlay after any change, there's no need for the evaporate property anymore, right?

[joaotavora]

There's one usage, but it occurs in a loop;

Setting two props instead of one is not going to make a difference :-)

create a list with a lambda in it only once.

Oy, now that I look carefully, that's a nono! just use a normal lambda

Also, just to make sure: if we delete the overlay after any change, there's no need for the evaporate property anymore, right?

Yes,

[astoff]

Okay, I updated the pull request.

Oy, now that I look carefully, that's a nono! just use a normal lambda

I presume you refer here to creating a closure/unquoting the lambda. If I may ask, for my own benefit, why it that a nono? In the previous version of the PR, where the lambda was created outside eglot--highlight-piggyback (hence only once) a closure is definitely better, no?

[joaotavora]

If I may ask, for my own benefit, why it that a nono?

Quoting lambda, you mean? Because it doesn't create a closure, and you lose lexical binding for one. The compiler is forced to generate a list that will create a different function object at runtime each time.

If you use lambda, the macro that expands to the special form function in a loop, the compiler picks up on it at compile time and will only create one function object. Only one function object exists for as many times as the loop is iterated, though multiple closures holding the different data that the code manipulates will potentially be created.

I guess if the compiler detects that the lambda doesn't need to capture anything it can skip creating the closure and just create a plain function.

I'm not a compiler/Lisp expert. @monnier can probably correct me on this.

[astoff]

This is why I originally created a category: it allowed moving the lambda outside of the function, so you don't need to be a compiler/Lisp expert to be sure only one function object (and only one closure) is created.

Also, note that I originally wrote `(,(lambda ...), not '((lambda ...)), i.e., I _un_quoted the lambda inside a list. The current version of the PR uses the second variant (because I thought that's what you said was preferred — did I misunderstand you?). I'd be curious to know what the implications are, both when these forms appears at the top level, and inside a function.

I'm not a compiler/Lisp expert.

Don't BS me!

[joaotavora]

Also, note that I originally wrote `(,(lambda ...), not '((lambda ...)), i.e., I _un_quoted the lambda inside a list. T

When you unquote it should be fine. As long as the compiler gets to "see" the function special form that is inside he expansion of the lambda macro. So maybe I don't understand the need for quoting at all. Ah I see, modification hooks takes a list. Is that it?

Anyway, don't worry about performance prematurely. Trust the compiler and write straightforward Lisp. Categories are for reusing code, not for performance optimizations.

[monnier]

`(,(lambda ...)) is the "right" way to do it, thanks, whereas '((lambda ...)) ends up quoting the function (turning it into a list).

[joaotavora]

@astoff

did I misunderstand you?).

More or less. I lent myself to be misunderstood :-) Your original version was fine lambda-quoting-wise indeed. I got confused by the double parenthesis and thought you were making a list, as Stefan pointed out. It's in your changed version that the problem I was pointing to actually appeared.

[astoff]

All right, should be good to merge now, and thanks for the free Lisp lesson.

[astoff]

@joaotavora Friendly ping