Skip to content

Add spinner#135

Open
Rid1FZ wants to merge 2 commits into
romus204:mainfrom
Rid1FZ:add-spinner
Open

Add spinner#135
Rid1FZ wants to merge 2 commits into
romus204:mainfrom
Rid1FZ:add-spinner

Conversation

@Rid1FZ

@Rid1FZ Rid1FZ commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

This commit adds an implementation of a spinner animation when installing any parser. This is the bare minimum implementation I could came up with with.

Here is a demo:

Screencast.From.2026-06-19.16-21-00.mp4

Rid1FZ added 2 commits June 19, 2026 16:10
calling `timer:stop` and `timer:close` when the timer is already closing
due to some reason can cause a race condition. So `timer:is_closing` check
has been added before `timer:stop` and `timer:close` calls

@tunaflsh tunaflsh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I understand your code correctly, you are not spinning dependencies, but only the language being explicitly installed. Why not query all languages being installed with installer.status after the async call to installer.install() and starts all spinners. Then in the callbacks stop those installed.

I am also thinking maybe I can include the language being installed in the callback output of the install function. Otherwise it could still work by checking installer.status.

@Rid1FZ

Rid1FZ commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

If I understand your code correctly, you are not spinning dependencies, but only the language being explicitly installed. Why not query all languages being installed with installer.status after the async call to installer.install() and starts all spinners. Then in the callbacks stop those installed.

I actually didn't thought of it. Let me check if I got this right,

Instead of relying on the spinning table, we can directly rely on the installer.status table to check what is being installed, and what is not. This way, we will be able to show spinner on the deps (by walking through the whole table). And, as the callback function is called once for every language after finishing the installation, we can add a check inside the callback function which will go through the whole installer.status to check which installations are done, and stop spinner for those.

Is it okay?

@tunaflsh

Copy link
Copy Markdown
Collaborator

Yes, exactly. But you still need the spinning table, or you won't be able to access it in the callback.

@tunaflsh

Copy link
Copy Markdown
Collaborator

Also this is gonna be quite a challenge to merge this with #132 where the lines can be added, removed or reordered.

@Rid1FZ

Rid1FZ commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Also this is gonna be quite a challenge to merge this with #132 where the lines can be added, removed or reordered.

Okay, as that pull request is ready to merge, I will rebase my local repo after the merge to prevent conflict, and commit the changes after that.

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