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

Make hunking block for consistent subsequent actions #1107

Merged
merged 1 commit into from Feb 21, 2019

Conversation

kaste
Copy link
Collaborator

@kaste kaste commented Feb 14, 2019

We must ensure that hitting 'h' very fast does run completely ordered, and
always waits for 'gs_diff_refresh' to complete. ('gs_diff_refresh' updates our
model.)

Please NOTE that running a command from the worker will still run that command
on the main thread. T.i. set_timeout_async(lambda: view.run_command("refresh"))
is misleading and a footgun because the "refresh" command here will still start
on the main thread. We basically just await the worker and then put a
task/command on the main worker queue.

Fixes #1104

We must ensure that hitting 'h' very fast does run completely ordered, and
always waits for 'gs_diff_refresh' to complete. ('gs_diff_refresh' updates our
model.)

Please NOTE that running a command from the worker will still run that command
on the main thread. T.i. `set_timeout_async(lambda: view.run_command("refresh"))`
is misleading and a footgun because the "refresh" command here will still start
on the main thread. We basically just await the worker and then put a
task/command on the main worker queue.

Fixes timbrel#1104
@randy3k
Copy link
Collaborator

randy3k commented Feb 20, 2019

I don't know if it is better to use sync operations or a lock.

@kaste
Copy link
Collaborator Author

kaste commented Feb 21, 2019

Sorry, don't know what you mean here. What are your concerns?

'Make hunking block' is maybe a bit short. What I actually do here is make the hunk action one task. Do not split it up into multiple tasks. That is so that you can put 'hhh' three hunking tasks on the queue, and each of them runs like a function from start to end.

It would be a misunderstanding that it is necessary to run these tasks on the main thread then. It is totally possible to run the hunking - each as one block/task - on the worker or any other thread. The reason to run it on the main thread is purely for UI/UX. It just looks and feels nicer if you do it on the main thread because we avoid intermediate redraws and yank.

@randy3k
Copy link
Collaborator

randy3k commented Feb 21, 2019

Ok, thank you for the explanations. I thought you wanted to wait for an action to be completed before triggering another h. And I was thinking that a lock may do a better job in this aspect.

@randy3k randy3k merged commit 77d1a69 into timbrel:dev Feb 21, 2019
@kaste kaste deleted the blocking-hunking branch February 21, 2019 14:49
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