Add onMouseEvent callback

[cutelisp]

This will enhance onAnyEvent, allowing it to capture event data that's otherwise inaccessible.

[dmaluka]

Ok, at least it wouldn't break compatibility with existing plugins (I thought it would, but I've just checked that it doesn't)...

I'm wondering what is the use case, and how exactly you are gonna use this tcell.Event type from Lua code. Could you share an example?

[cutelisp]

I wan't to be able to know the cords of the cursor while dragging. Afais, those values aren't exposed to lua side. I think it's more convenient to expose the event to onAnyEvent than on the actions callbacks.

function onAnyEvent(event)
    local ex, ey = event:Position()
    ...
end

[dmaluka]

local ex, ey = event:Position()

You are assuming that the event is EventMouse?
Did you actually try to use that?

I wan't to be able to know the cords of the cursor while dragging. Afais, those values aren't exposed to lua side.

We could expose them.

[JoeKar]

Yes, the identification of all of the different event types could be quite hard in this case. With this PR you will have the access to all and everything, but need to take care to access the right type/memory.

[cutelisp]

You are assuming that the event is EventMouse? Did you actually try to use that?

Yes, I've tried it and it works (not heavily tested tho). Maybe I should've sent a better snippet.

Maybe not the most neat logic, but this was my approach.

clicking = false 

function onMousePress()
    clicking = true 
end

function onMouseRelease()
    clicking = false 
end

function onAnyEvent(event)
    if clicking and event then
        local ex, ey = event:Position()
        ...
    end
end

[cutelisp]

We could expose them.

Right, I think this would be a great addition anyway

[Andriamanitra]

I don't think exposing tcell.Event directly is the best approach. To access anything useful plugins will need to somehow figure out the concrete type of the event which is not trivial to do in Lua (@cutelisp's implementation above crashes if the user happens to press a key while holding down a mouse button). It also poses difficulties for documentation/maintenance because the types live in a separate 3rd party package (although I guess micro-editor/tcell has already somewhat diverged from the upstream tcell).

As a plugin author I would rather have new "event callbacks" ¹ with more specific arguments and stronger backwards compatibility guarantees, eg. something along the lines of onMouseDrag(x, y, button) and onMouseClick(x, y, button).

Footnotes

1. @dmaluka brought up the idea of adding more of these in https://github.com/zyedidia/micro/issues/3550#issuecomment-2508735791 ↩

Remove approval due to type non-safety within the Lua callbacks.
This would only work in case we expose a further API to check the types we give as callback arg.

[JoeKar]

[...] eg. something along the lines of onMouseDrag(x, y, button) and onMouseClick(x, y, button).

I'd prefer something like onMouseEvent(MouseEvent), which should include key/button, press, release and positions etc.pp.
I'm no friend of having a (callback-)function for every single bit, but I'm not a plugin developer...

[cutelisp]

[...] eg. something along the lines of onMouseDrag(x, y, button) and onMouseClick(x, y, button).

I'd prefer something like onMouseEvent(MouseEvent), which should include key/button, press, release and positions etc.pp. I'm no friend of having a (callback-)function for every single bit, but I'm not a plugin developer...

Since the use case by now is just for mouse events maybe we should go this way.
I prefer that idea over @Andriamanitra one.

We could create onMouseEvent(EventMouse) since EventMouse already have methods for all those things, the only info it doesn't provide have is the state (drag, click or release) it can be achieved by logic on lua side tho.
MouseEvent have a state atribute, not sure if would be neat to have MouseEvent and EventMouse both as args

[cutelisp]

Changed the PR towards this suggestion #3774 (comment)
Tried to make things simple and just pass the essential args.
I'll appreciate feedback.

[JoeKar]

I don't think that it is a good idea to export values/definitions from tcell's interface.
Shouldn't we wrap it?

[cutelisp]

What do you mean by wrap it, could you give an example?

[JoeKar]

By exporting the content of e.Buttons() and e.Modifiers() we implicit export a dependency to the interface of tcell.
If this is OK, then we can go that way, but I've some doubts right now.
The plugins should only have a dependency to micro, but not its dependencies. To solve this we would need to define our own MouseEvent, which we already have (with tcell dependencies too)...
So a new struct could fit the needs and it should include the coordinates, buttons and modifiers. On the other hand this means, that we duplicate this kind of event.

@dmaluka:
What is your opinion about it?

[dmaluka]

Actually we already export *tcell.EventMouse to lua in some cases, namely, we pass it to lua functions bound to mouse events (MouseLeft, MouseRight, MouseRightDrag etc), since #2605. Yeah, I'm guilty of that. (Also I'm guilty of not documenting that.)

So, we may debate whether or not it is/was a good idea to directly expose tcell stuff to lua... But since we are already doing that anyway, I think, at least, that simply passing the entire *tcell.EventMouse is in all respects better than passing its components as individual arguments.

...But first of all, I'm not sure we want this new callback in the first place. It seems what we actually want is to pass the mouse info (for example, again, just pass *tcell.EventMouse) to the already existing callbacks of "mouse actions", i.e. to onMousePress, onMouseDrag, onMouseRelease, onMouseMultiCursor, preMousePress, preMouseDrag, preMouseRelease, preMouseMultiCursor.

And that seems easy to implement: #3779

[JoeKar]

But since we are already doing that anyway, I think, at least, that simply passing the entire *tcell.EventMouse is in all respects better than passing its components as individual arguments.

Agree.

[dmaluka]

#3779 merged, can we close this PR as unneeded?

[cutelisp]

#3779 merged, can we close this PR as unneeded?

Absolutely, thanks for your PR.