-
Notifications
You must be signed in to change notification settings - Fork 68
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
feat: [#387] Enhance the ability of the command feature #476
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #476 +/- ##
==========================================
- Coverage 71.34% 70.30% -1.04%
==========================================
Files 171 172 +1
Lines 10388 10538 +150
==========================================
- Hits 7411 7409 -2
- Misses 2410 2562 +152
Partials 567 567 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can test the logic? And could you add some screenshots for this feature in description?
I have tested it locally (works great) and will include screenshots. Additionally, I will try to test it programmatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great optimization
return answer, nil | ||
} | ||
|
||
func (r *CliContext) Spinner(message string, option ...console.SpinnerOption) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to use this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an example for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually spinner is different from progressBar. I will try to implement it similar to image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 👍
console/progress_bar.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0s% shouldn't be there. I didn't get anything like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can try this:
ctx := &console.CliContext{}
bar := ctx.CreateProgressBar(10)
err := bar.Start()
if err != nil {
ctx.Error(err.Error())
return
}
for i := 0; i < 100; i++ {
// performTask()
if i%2 == 0 {
bar.Advance(2)
} else {
bar.Advance()
}
time.Sleep(time.Millisecond * 50)
}
err = bar.Finish()
if err != nil {
ctx.Error(err.Error())
return
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's because of the code mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work
Closes goravel/goravel#387
📑 Description
Example
Ask
Confirm
Secret
MultiSelect
Spinner
ProgressBar
✅ Checks
ℹ Additional Information