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: gather sources does not cancelled on refresh #132

Merged
merged 3 commits into from
Dec 29, 2024

Conversation

Milly
Copy link
Contributor

@Milly Milly commented Dec 28, 2024

Enabled cancellation of gather sources by locking only the UI during refresh, not the entire refresh process.

This PR includes 3 cumulative commits. If necessary, these can be separated into individual PRs.

Implement a signal reason as Error object.
Added a cancel() method to GatherState, instead of providing signal to
constructor.
Enabled cancellation of gather sources by locking only the UI during
refresh, not the entire refresh process.
@Shougo
Copy link
Owner

Shougo commented Dec 28, 2024

@Milly #123 can be closed?

@Shougo
Copy link
Owner

Shougo commented Dec 28, 2024

I will test it.

@Milly
Copy link
Contributor Author

Milly commented Dec 28, 2024

#123 can be closed?

Yes, it is contained in this PR.

@Shougo
Copy link
Owner

Shougo commented Dec 29, 2024

OK.

@Shougo Shougo mentioned this pull request Dec 29, 2024
@Shougo Shougo merged commit 54248fb into Shougo:main Dec 29, 2024
2 checks passed
@Shougo
Copy link
Owner

Shougo commented Dec 29, 2024

Thanks. Merged.

@Shougo
Copy link
Owner

Shougo commented Jan 7, 2025

I have found the bug.

If I -resume the sources, redraw does not work until force refresh items.

this.#aborter.abort({ reason: "quit" });
const reason = new QuitAbortReason();
this.#aborter.abort(reason);
/* no await */ this.#cancelGatherStates([], reason);
Copy link
Owner

Choose a reason for hiding this comment

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

This is broken.

Copy link
Owner

Choose a reason for hiding this comment

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

It must not be canceled. Because if it is canceled, resume feature does not work.

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