Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Added unicode progress bar #134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mickvav
Copy link

@mickvav mickvav commented Jan 29, 2023

Nice book to read, thanks @dylanaraps !

This PR contains a little trick of mine that combines unicode characters into smoother variant of progress bar. Sending PR in hope you will consider it a meaningful contribution to a book.

```sh
function progress () {
local symbols=(" " "\u258F" "\u258E" "\u258D" "\u258C" "\u258B" "\u258A" "\u2589" "\u2588")
local n=$(($1/8))

Choose a reason for hiding this comment

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

Why not define 8, which is used multiple times, dynamically like e.g. length=$((${#symbols[@]}-1))?

Copy link
Author

@mickvav mickvav Jan 30, 2023

Choose a reason for hiding this comment

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

There is no chance that unicode character table will change in future to have something else, thus 8 is a true constant here.
In the same time, determining array length on every function call will eat some CPU on every execution and give no value.

(although I completely agree with gut instinct of "constant that is > 1 should be named and computed once" - this is very good default assumption that makes perfect sense in 90-99% cases)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants