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 spinner style settings #1490

Closed
wants to merge 1 commit into from
Closed

Conversation

KaiSforza
Copy link

Allows for customizing the spinner style finally! Previously it had a problem where the attribute name was not _style_level_spinner, so it was never getting set from the initialize_styles method. This made the setting in the user's config.toml never see the light of day.

To fix this I also changed all of the style_level_{level} names to be just style_{level}, cleaning up a bit of the code.

And for some reason the Spinner class doesn't keep a name attribute, so I'll be poking the rich project about that in a minute, since I basically had to create a Spinner() class with no ability to use it.

@Grub4K
Copy link
Contributor

Grub4K commented May 15, 2024

See also #1471 (my PR) for a different implementation. I have the same issue but @ofek didn't yet look at that PR either.

@KaiSforza
Copy link
Author

@Grub4K Nice, I hadn't seen that. Hopefully these get looked at. Yours is a bit simpler, but I figured that the project might as well get rid of the _style_level_X stuff in favor if _style_X, since that maps on much more nicely to the configuration options.

Also was hoping to get Textualize/rich#3359 merged so we could have a more similar workflow, as well. Then we could just have

parse_style = lambda s: Spinner(s).name if option == 'spinner' else Style.parse(s)

and remove all the extra conditional stuff.

@ofek
Copy link
Sponsor Collaborator

ofek commented May 16, 2024

I'm so sorry for not responding! I'm getting ready to travel for my talk at PyCon and I won't have time until next week.

Allows for customizing the spinner style finally! Previously it had a
problem where the attribute name was not `_style_level_spinner`, so it
was never getting set from the `initialize_styles` method. This made the
setting in the user's `config.toml` never see the light of day.

To fix this I also changed all of the `style_level_{level}` names to be
just `style_{level}`, cleaning up a bit of the code.

And for some reason the `Spinner` class doesn't keep a `name` attribute,
so I'll be poking the rich project about that in a minute, since I
basically had to create a `Spinner()` class with no ability to use it.
@ofek
Copy link
Sponsor Collaborator

ofek commented May 23, 2024

I merged the other PR so there is now a conflict, sorry about that!

@KaiSforza
Copy link
Author

@ofek No worries, I think I'll throw in some quick changes on @Grub4K's changes. Thought of some ways to make it a bit cleaner.

@KaiSforza KaiSforza closed this May 23, 2024
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

3 participants