Skip to content
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

Fix searching the last command output #4522

Merged
merged 5 commits into from
Jan 15, 2022

Conversation

page-down
Copy link
Contributor

When searching the output of the last command executed, if scrollback has been scrolled,
then the start point is actually outside (below) the screen and the number of scrolled lines needs to be added.

That's my bad. I don't know how this was slipped through. :(

Please review, thank you.

@page-down
Copy link
Contributor Author

In a previous change to fix mark duplication, some code was removed.
But today I tested it and found that the output continued is occurring.
I'll try to add it back to see if it fixes the related issue.

2cc3922

@page-down
Copy link
Contributor Author

After I added the code back in, I could get the command output normally after resizing the window.

I did not submit this part yet. Maybe need to look at the previous fix first.

I found that the prompt mark appears to be copied.

c43637f

@page-down
Copy link
Contributor Author

Not really, it has nothing to do with the previous fix.

I pushed a new commit and checked the continued attribute.
Currently getting the output normal after resizing the window.

Please let me know if there is anything missed.

@kovidgoyal
Copy link
Owner

I wont be able to review this till tomorrow, however I think you should consider adding some tests for it as this is relatively easily testable and fairly complex. See the test_prompt_marking() function

@page-down
Copy link
Contributor Author

Sure, I'll add more tests.
I've always thought kitty's test cases could be covered a bit more.
Apart from system integration (especially macOS), isn't it possible to cover a bit more range? How would it be more appropriate to contribute?

@kovidgoyal
Copy link
Owner

You are welcome to send PRs with more tests, if you need help getting
started, just ask.

@kovidgoyal kovidgoyal merged commit 725ddf4 into kovidgoyal:master Jan 15, 2022
@page-down page-down deleted the fix-ksi-last-cmd branch January 16, 2022 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants