-
-
Notifications
You must be signed in to change notification settings - Fork 996
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
somehow ctrl-c fails if copy_or_interupts is set and breaking out of an infinite loop #4713
Comments
|
kitty/screen.c static PyObject*
extend_tuple(PyObject *a, PyObject *b) {
Py_ssize_t bs = PyBytes_GET_SIZE(b);
// ... The parameter b should be tuple here, why do we use |
What should I do to get a situation where static PyObject*
text_for_selections(Screen *self, Selections *selections, ... ) |
You cannot kitty does not currently support multi-select. Though IIRC |
Steps to reproduce:
static PyObject*
text_for_range(...) {
IterationData idata;
iteration_data(self, sel, &idata, -self->historybuf->count, false);
int limit = MIN((int)self->lines, idata.y_limit);
// lines: 5, y_limit: -28, y: -24 (min_y: -24)
// -28 - (-24) -> PyTuple_New(-4)
PyObject *ans = PyTuple_New(limit - idata.y); To fix this: static void
iteration_data(const Screen *self, const Selection *sel, IterationData *ans, int min_y, bool add_scrolled_by) {
// ...
ans->y = MAX(ans->y, min_y);
ans->y_limit = MAX(ans->y_limit, ans->y);
} Maybe there are problems in other places too. |
I'm not a fan off invalidating selections on scroll, one then has to check |
The only reason I bring this up is that I see My intention is to reduce the unnecessary checks and only do it when needed. |
On Sat, Feb 19, 2022 at 11:51:04PM -0800, page-down wrote:
> ... one then has to check the selection at every scroll, and scrolling can be quite fast ...
The only reason I bring this up is that I see `iteration_data` being called on every scroll, and even on every mouse-move event, when there is a selection.(regardless of whether the selection is on-screen or not)
If that's the case then I have no objection to removing the selection if
iteration_data() indicates it is empty on scroll.
|
It looks like the text selection is iterated over (not the content, just If you don't have time, I need to understand the possible impacts involved before I could touch the code. Should However, when this selection is rolled over several screens (still in the history range), the user may have forgotten and would like to interrupt. |
On Sun, Feb 20, 2022 at 04:11:13AM -0800, page-down wrote:
It looks like the text selection is iterated over (not the content, just `iteration_data`) when rendering each frame, not just when moving the mouse or scrolling.
Is it possible to ignore the text selection when it is not on screen and the subsequent rendering will quickly skip the check? (Perhaps the performance gain is not worth it.)
That kind of micro optimisation should be benchmarked, I dont think
calculating iteration data is at all expensive.
If you don't have time, I need to understand the possible impacts involved before I could touch the code.
Yeah this is not worth the time investment to me.
---
Should `copy_or_interupts` interrupt the program when the selected text is not on screen?
I don't think so. In principle, the text should be copied because there is an invisible text selection.
Since no one has asked for this, lets not create unneccessary work for
ourselves :)
|
somehow ctrl-c fails if breaking out of an infinite loop
I'm not sure on what is actually relevant so I'll just post the function that I was trying to escape from
I had actually ran this 1-2 times before this, I was trying to debug an issue binding to sockets in a udp client/server program. I'm not sure why it worked once or twice before printing output in (unintentionally) never ending loop, but something is causing it to fail.
To Reproduce
Steps to reproduce the behavior:
Screenshots
If applicable, add screenshots to help explain your problem.
Environment details
Additional context
This obviously only seems relevant if copy_or_interrupt is set, so it makes no sense to use a default config
The text was updated successfully, but these errors were encountered: