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

the function to display bars of given percentage from given width #478

Merged
merged 8 commits into from
May 12, 2023

Conversation

maxim-uvarov
Copy link
Contributor

for $i in 1..10 {$i / 10 | print (bar $in)}
▌
█
█▌
██
██▌
███
███▌
████
████▌
█████

@maxim-uvarov maxim-uvarov changed the title function to display bars of given percentage from given width the function to display bars of given percentage from given width May 6, 2023
Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

now that is quite amazing 😮 👏 👏

transcript

let n = 10000
for $i in 1..$n {
    $i / $n | print -n $"(bar $in 50)\r"
}

💡 what about allowing the following syntax to achieve the same? 😋

let n = 10000
for $i in 1..$n {
    $i / $n | bar $in --width 50 --progress
}

@fdncred
Copy link
Collaborator

fdncred commented May 6, 2023

There are some progress bars that are similar to this https://github.com/nushell/nu_scripts/tree/main/sourced/progress_bar

@maxim-uvarov
Copy link
Contributor Author

There are some progress bars that are similar to this https://github.com/nushell/nu_scripts/tree/main/sourced/progress_bar

I tried all of them several months ago, but none of them satisfied my needs. As I went through their code now, I can confirm that none of those functions can be used to produce bars in the cells of some column. Therefore, I think that my PR is still legitimate.

@maxim-uvarov
Copy link
Contributor Author

now that is quite amazing 😮 👏 👏

🙏
@amtoine, Your gif is 🔥🔥🔥! Animation without using pauses in nushell, never saw this!

@maxim-uvarov
Copy link
Contributor Author

💡 what about allowing the following syntax to achieve the same? 😋

I will try to implement

@maxim-uvarov
Copy link
Contributor Author

@amtoine

💡 what about allowing the following syntax to achieve the same? 😋

Like this? 05859b8

@amtoine
Copy link
Member

amtoine commented May 6, 2023

looks nice 😮

@fdncred
Copy link
Collaborator

fdncred commented May 6, 2023

There's also the sparkline thing in I wrote. https://github.com/nushell/nu_scripts/blob/09647b77ee0e127f1b677e7e49652ac2c82dd2d6/sourced/fun/spark.nu

Not saying we can't land yours. Just pointing out similar things.

@maxim-uvarov
Copy link
Contributor Author

There's also the sparkline thing in I wrote.

Wow!!! I saw it already, but forget about it! I hope I will find some applications soon, as I am a big fan of excel sparklines. 🙏

@maxim-uvarov
Copy link
Contributor Author

I added some tests by analogy with a sparkline, though I needed to use escape codes for result testing. Not sure if it is ok. Please, let me know if I can enhance the function, or tests somehow. I'm glad to learn.

@fdncred
Copy link
Collaborator

fdncred commented May 7, 2023

49;39 is using the default color for foreground and background iirc. it's weird that you'd have to use this. i wonder if this is a issue local to your system/terminal.

Emily Grace Seville and others added 2 commits May 7, 2023 15:00
@maxim-uvarov
Copy link
Contributor Author

I apologize for not checking if escape codes were necessary while using 'pbcopy'. Additionally, I fixed another error.

And by mistake I merge new commits from the main branch into my branch with the PR changes.:-(

@maxim-uvarov
Copy link
Contributor Author

I appologize again. It's late and I'm making mistakes: now something is wrong with my tests. I'll fix everything some other day

Fixed tests, although they need escape sequences, as the bar function has the ability to change the colors of both the background and foreground and uses the default color by default.
@EmilyGraceSeville7cf
Copy link
Contributor

@maxim-uvarov, what about gradient bars?

@maxim-uvarov
Copy link
Contributor Author

@EmilySeville7cfg Sounds cool! I will check how they look with the next approach.

@fdncred
Copy link
Collaborator

fdncred commented May 11, 2023

Are we done here? I still don't really understand this

def bar_tests [] {
    assert_eq 1 "�[49;39m▏    �[0m" 0.03
    assert_eq 2 "�[49;39m▎         �[0m" 0.03 10
    assert_eq 3 "�[49;39m▊�[0m" 0.71 1
    assert_eq 4 "�[49;39m███████   �[0m" 0.71 10
}

I mean i know what it says but i'm not sure why it needs escape codes.

@maxim-uvarov
Copy link
Contributor Author

maxim-uvarov commented May 12, 2023

I still don't really understand this

@fdncred As far as I can understand, the function is able to customize the colors of bars and their background. Its output uses escape codes, and default colors are used as defaults in the function. https://github.com/maxim-uvarov/nu_scripts/blob/3b1fc0fbb35b1a5def471292c66d54fa5632f3d3/sourced/progress_bar/bar_fn.nu#L27

Sometime ago, I tried to remove escape codes, but without them, tests won't work.

Are we done here?

As I can see from my side - the function works properly. Maybe it is meaningful to rename the file to just 'bar', but I am not sure.

@fdncred
Copy link
Collaborator

fdncred commented May 12, 2023

Ahh, thanks for the link. I was missing where you were assigning colors, even though it's a default, it' still a color.

Ya, If you rename it to bar.nu I'll go ahead and land it.

@maxim-uvarov
Copy link
Contributor Author

Ya, If you rename it to bar.nu I'll go ahead and land it

done

@fdncred fdncred merged commit b2fadc0 into nushell:main May 12, 2023
@fdncred
Copy link
Collaborator

fdncred commented May 12, 2023

thanks

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.

4 participants