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 toggle task list display #390

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EasonMo
Copy link

@EasonMo EasonMo commented Dec 24, 2024

When toggling display task list, it will only be displayed at the end of a large number of blank lines, and will not correctly display the effect of executing util.terminal_tail_hack().
image
The reason is that the execution order is wrong. The solution is to delay setting the buffer.

@github-actions github-actions bot requested a review from stevearc December 24, 2024 18:21
Copy link
Owner

@stevearc stevearc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix makes me a little nervous because it introduces some possible race conditions (what if the window is invalid? What if the buffer is deleted? What if something else switched this window's buffer?) Could you either find a way to fix it without the defer, or provide some repro steps so that I can investigate it myself?

@EasonMo EasonMo force-pushed the fix_toggle_display_terminal branch from 3ec11db to 7a2efdd Compare December 25, 2024 00:17
@github-actions github-actions bot requested a review from stevearc December 25, 2024 00:18
@EasonMo
Copy link
Author

EasonMo commented Dec 25, 2024

Hi, I was a little careless. Now I have analyzed it carefully. The reason why it cannot be displayed correctly is that when calculating scroll lines, the SideBar window has not been generated yet, and terminal_tail_hack() has not been successfully executed. Therefore, TaskView:update() should be executed after the SideBar window is generated, rather than in TaskView:new() .What do you think?

@EasonMo
Copy link
Author

EasonMo commented Dec 30, 2024

Are there any updates regarding these questions?

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.

2 participants