cybardev / cutefetch

Tiny coloured fetch script with cute little animals
https://cutefetch.cybar.dev
GNU General Public License v3.0
44 stars 4 forks source link

feat: add randomizer function #9

Open cybardev opened 10 months ago

cybardev commented 10 months ago

Description

We want a function in the script that uses shuf to randomize the animals and eyes. Currently we only have a note under the Extras section in the readme.

Code suggested by @Kalitsune in https://github.com/cybardev/cutefetch/issues/8#issuecomment-1813084243:

# welcome screen
fetch () {
  eyes=(0 1 2 3 4 6 7 8 11 12 14)
  cutefetch -k2 $(shuf -e "${eyes[@]}" -n 1)
}
fetch

This radomizes the eyes but not the animal. Ideally, we would also randomize the animal. Optionally, we can make it a flag with args, like -r eyes and -r animal, with the default -r and -r all having both effects.

Changes Requested

Acceptance Criteria

Blockers

Kalitsune commented 10 months ago

Suggestion: Replacing the --animal argument by an option:

cutefetch <animal> <eyes>

(It doesn't make sense to have a --flag for this if you cant print two animals at once)

And accepting arrays as input:

cutefetch (bunny,kitty,doggy) (0, 1, 5, 12)

And if an array is passed as an input, have the program choose a random value in it

Of course, this could still be used without using the random feature like this:

cutefetch kitty2 12
cybardev commented 10 months ago

That makes a lot of sense. Thanks for the suggestions, @Kalitsune. I think changing the flags to args should be another issue since this issue is just about the randomizer, so I'll make a separate issue on that sometime soon.

Also, we should probably still have a "randomize from all" flag or argument, or maybe if <animal> and <eyes> are given "random" then we randomize that. Cuz it'd be cumbersome to list all available animals and eyes in args.

Kalitsune commented 10 months ago

Sure. But if we're planning on changing the args its probably better not to start implementing the random yet to avoid having to reimplement it once the change is done

And yes, using "random" as an arg for randomize all makes a lot of sense we should do that!

cybardev commented 10 months ago

Alright. In that case, you can hold off making the randomizer for now in case you were planning to work on it.

I'll make a PR with the new argument parsing system and let you know when it's ready.

Thanks for your help and insight on these issues. Enjoy your day(s) 😊

Kalitsune commented 10 months ago

Thanks! You too! Good working on it!

On Thu, Nov 16, 2023, 6:07 PM Sheikh @.***> wrote:

Alright. In that case, you can hold off making the randomizer for now in case you were planning to work on it.

I'll make a PR with the new argument parsing system and let you know when it's ready.

Thanks for your help and insight on these issues. Enjoy your day(s) 😊

— Reply to this email directly, view it on GitHub https://github.com/cybardev/cutefetch/issues/9#issuecomment-1814869136, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARVE2BNIXJCD4E3ZZPFFJT3YEZB3JAVCNFSM6AAAAAA7OFGXL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUHA3DSMJTGY . You are receiving this because you were mentioned.Message ID: @.***>