-
-
Notifications
You must be signed in to change notification settings - Fork 681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial mouse support #448
Conversation
Hey @tlinford - happy to see you taking this up! |
Hi @imsnif , We can implement text-marking ourselves for sure, but as you say it's going to be an advanced thing with quite a lot of work to get it going Also, are you thinking of having mouse support always enabled, or should it be a toggle? |
Right. The issue is that we can't publish a version that doesn't allow text-marking. I think it will kind of render the app unusable (not being able to copy/paste from the terminal). Maybe there's some sort of workaround, or?
That would be very cool!
IMO it should be always enabled. |
Agreed, the only thing that I've found for now is that shift click bypasses mouse capture so that can be used. Haven't found a way to capture the events and let them pass through yet. |
If it's a termion issue, I am very open to looking at other solutions (eg. crossterm) if it helps... |
I could easily be wrong about this, as I don't really know much about terminal escapes codes, but from what I found it's not really a termion thing. Once the csi sequence escape codes (termion uses csi!("?1000h\x1b[?1002h\x1b[?1015h\x1b[?1006h")) to enable mouse reporting are written, shift click is the only way to inhibit that behaviour. |
Good find! I played a little bit with this sequence and it seems that the one part of it that actually stops mouse reporting tot he terminal is So if this is not something that's possible with termion, maybe it's possible with crossterm? Or something else? Or maybe we can somehow hack around this by immediately cancelling the |
ba66136
to
f030bf6
Compare
54fabf5
to
3fb73fb
Compare
Hey @imsnif,
The text selection is still buggy when selecting across multiple panes, but I'd appreciate some feedback on the direction this is going |
Hey @tlinford - I didn't have a whole lot of time today to take a deep look at this. I plan on doing so tomorrow. Meanwhile my thoughts on copy-paste: I would recommend going in a different direction. The problem with using something that interacts directly with the clipboard is that aside from having to support many different platforms, we would encounter problems when Zellij is run on a remote server. There is a terminal signal that tells the terminal emulator to place something in the clipboard (OSC 52). Meaning we'd be sending it to stdout and the terminal emulator would do the rest. I only did some brief searching for it, and found this generally related article: https://jdhao.github.io/2021/01/05/nvim_copy_from_remote_via_osc52/ So if you're having trouble finding the exact syntax of the signal let me know and I'll do some more thorough research. |
@tlinford - I took a deeper look and also tried this. It looks really good. As you said, still a little rough but I'm very happy to see where this is going. A behaviour I noticed that needs changing: it's possible to select text in more than one pane at the same time (not sure if this is what you meant with the multiple pane bugginess?) Other than that, let me know if you want a hand with the OSC stuff. Let me know when you're done and I'll do a thorough go-over of the code (though at a glance things seem fine). |
Oh! One more thing: I'm doing some work on the render function to improve its performance. Considering that, it might actually be better to handle the character selection in the grid and not in the render function. I know I said otherwise on discord - sorry about that :) The reason for this is that the optimization I'm doing would mean that only changed characters will be sent to the terminal render function from the grid - so if you do it that way, the merge will be less painful I think. I hope that doesn't throw a wrench in your plans? |
Glad to hear that you are happy with things so far :)
|
Don't work too hard on this (or maybe leave it to the end?) - I'm in very initial stages of the performance improvements and things might change. |
fc5767f
to
4c81bad
Compare
@imsnif Quick update:
I realized that the current approach is a bit limited, as the selection is relative to the viewport, and not the whole terminal's text buffer, so for example holding left mouse button, and scrolling up/down won't behave as one would expect at the moment. |
Sounds good! Sorry, I just realized maybe you're stuck because of me? |
Not stuck, no worries! Will try and start moving the selection to Grid in the next days and see how things go. |
- Try to minimize lines to update on selection start/update/end
- the previous optimization to grid selection rendering was always adding the lines at the extremes of previous selection, causing problems when scrolling up or down - make sure to only add lines in viewport
- remove the useless methods from Impl Pane in PluginPane
- change repeater to take an action instead of a position with hardcoded action
Hi @imsnif, I migrated the tests to the new e2e setup (very cool!), the action repeater is now in |
Hey @tlinford - this looks great. Thanks for all the changes. I think I'm ready to merge this into |
Amazing :) So thanks very much for your incredible hard work and perseverance in implementing this feature. You've done a really great job! |
This is a very initial WIP for #175 now, with quite a few things that need discussing in more detail, but I figure it's a start: