AshlinHarris / Spinners.jl

Command line spinners in Julia with Unicode support
MIT License
13 stars 1 forks source link

Added @sindresorhus spinners #52

Closed curio-sitas closed 1 year ago

curio-sitas commented 1 year ago

Contribution to #36

Bug

Some of sindresorhus's spinners have wrong behaviors, review needed to correct them

AshlinHarris commented 1 year ago

I don't think the failed CI checks are due to your contributions. I'll look over everything and bypass the branch protection, if needed.

Sorry the testsets and CI are so messy. I'm waiting for interactive threadpools to be available before I do any major rewrites (#50).

curio-sitas commented 1 year ago

No problems, i'm still working in the PR so it doesn't need to be merged for now. And some spinners have strange behaviors.

curio-sitas commented 1 year ago

I think this can be merged.

There are misfunctionning multi-character spinners (e.g. :fistBump :mindblown) but i dont think it comes from their definitions but rather from the backend. Please take a look at that. I also had a scope problem :

for x in keys(Spinners.SPINNERS)

    @spinner x 

end

There is nothing printed in the console.

AshlinHarris commented 1 year ago

I also had a scope problem :

for x in keys(Spinners.SPINNERS)

    @spinner x 

end

There is nothing printed in the console.

I think this is related to #14. The macro doesn't parse variables correctly, so there's currently no way to save a String (or Vector{String} or Symbol) to a variable x and then pass x to the macro.

AshlinHarris commented 1 year ago

There are misfunctionning multi-character spinners (e.g. :fistBump :mindblown) but i dont think it comes from their definitions but rather from the backend. Please take a look at that.

I was able to replicate the issue:

pkg> activate --temp
pkg> add https://github.com/curio-sitas/Spinners.jl#new_spinners
julia> using Spinners
julia> @spinner :fistBump
julia> @spinner :mindblown

Both of these are the only spinners defined with "\u3000".

This reminds me of #13, but I'm not sure it's related. That issue deals with the final cleanup step for characters with varying transcode lengths, but this behavior happens continually. I'm assuming that the correct number of backspaces for "\u3000" isn't being calculated properly.

AshlinHarris commented 1 year ago

New issue #53

AshlinHarris commented 1 year ago

Thank you, @curio-sitas! I'll merge this regardless of any issues with the backend. Do you mind making sure the LICENSE is up to date? You may add yourself and any contributors for these spinners.

curio-sitas commented 1 year ago

Hey, Thank's for the proposition, i'll do that tomorow. I'll continue to work to contribute to the package since i want to learn more about utf encoding

AshlinHarris commented 1 year ago

Thanks, it means a lot that you're willing to help, especially in the package's messy early days. By the way, do you mind if I add you as a collaborator with an administrative role?

curio-sitas commented 1 year ago

Thanks, it means a lot that you're willing to help, especially in the package's messy early days. By the way, do you mind if I add you as a collaborator with an administrative role?

Sure you can !