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.

Sign up for GitHub

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

Fix: Bug where one progress bar display is missing. #52

Merged
merged 2 commits into from
Feb 3, 2024
Merged

Fix: Bug where one progress bar display is missing. #52

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cfed52e
Fix: Bug where one progress bar display is missing.
nao1215 Feb 3, 2024
1fd8a10
Intialize index before starting progress bar
nao1215 Feb 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ localstack
/cfn
.envrc
/t-rec*
/s3hub-download
8 changes: 7 additions & 1 deletion ui/s3hub/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,13 @@ func (m *s3hubDeleteBucketModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
}
m.sum = len(m.targetBuckets) + 1
m.status = statusBucketDeleting
return m, tea.Batch(m.spinner.Tick, deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))

return m, tea.Batch(
m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
Comment on lines +137 to +143
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The progress bar's percentage calculation does not account for the initial state before any deletion starts, potentially leading to an inaccurate display. Consider initializing the progress bar to reflect the starting state accurately.

+ m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
  progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(
m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(
m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))

Copy link
Owner Author

Choose a reason for hiding this comment

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

I Fix it. See 1fd8a10

Copy link
Contributor

Choose a reason for hiding this comment

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

@nao1215 Thank you for addressing the feedback and making the necessary changes. To ensure the fix is correctly implemented, please verify that the m.index = 0 initialization is placed accurately before the progress bar's percentage calculation in the ui/s3hub/delete.go file. This adjustment is crucial for accurately reflecting the starting state of the progress bar before any deletion begins. If everything is in order, this should resolve the issue with the progress bar's display accuracy.

}
case " ":
if m.status == statusBucketListed {
Expand Down
29 changes: 25 additions & 4 deletions ui/s3hub/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,13 @@ func (m *s3hubListBucketModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
}
m.sum = len(m.targetBuckets) + 1
m.status = statusDownloading
return m, tea.Batch(m.spinner.Tick, downloadS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))

return m, tea.Batch(
m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
downloadS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
Comment on lines +129 to +135
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The progress bar's percentage calculation does not account for the initial state before any download starts, potentially leading to an inaccurate display. Consider initializing the progress bar to reflect the starting state accurately.

+ m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
  progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(
m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
downloadS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(
m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
downloadS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))

Copy link
Owner Author

Choose a reason for hiding this comment

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

I Fix it. See 1fd8a10

Copy link
Contributor

Choose a reason for hiding this comment

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

@nao1215 Thank you for the update! To confirm the fix, please ensure that the m.index = 0 initialization is correctly placed before the progress bar's percentage calculation in the ui/s3hub/list.go file as per the commit 1fd8a10. This initialization is crucial for accurately reflecting the starting state of the progress bar before any download begins. If everything is in order, that should address the issue I raised. Great work!

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

}
case "D":
if m.status == statusBucketListed {
Expand All @@ -135,7 +141,12 @@ func (m *s3hubListBucketModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
}
m.sum = len(m.targetBuckets) + 1
m.status = statusBucketDeleting
return m, tea.Batch(m.spinner.Tick, deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))

return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
Comment on lines +146 to +151
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The progress bar's percentage calculation does not account for the initial state before any deletion starts, potentially leading to an inaccurate display. Consider initializing the progress bar to reflect the starting state accurately.

+ m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
  progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))
m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetBuckets[0]),
deleteS3BucketCmd(m.ctx, m.app, m.targetBuckets[0]))

Copy link
Owner Author

Choose a reason for hiding this comment

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

I Fix it. See 1fd8a10

Copy link
Contributor

Choose a reason for hiding this comment

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

@nao1215, thank you for addressing the feedback and making the necessary changes. To ensure everything is in order, could you please confirm if the initialization of the progress bar's index to accurately represent the starting state before any deletion starts has been added as suggested? If you've followed the suggestion accurately, then everything should be set correctly. 🐇

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

}
case "enter":
if m.status == statusReturnToTop || m.status == statusDownloaded || m.status == statusBucketDeleted {
Expand Down Expand Up @@ -432,7 +443,12 @@ func (m *s3hubListS3ObjectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
}
m.sum = len(m.targetS3Keys) + 1
m.status = statusDownloading
return m, tea.Batch(m.spinner.Tick, downloadS3ObjectsCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))

progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetS3Keys[0]),
downloadS3ObjectsCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))
Comment on lines +449 to +454
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The progress bar's percentage calculation does not account for the initial state before any download starts, potentially leading to an inaccurate display. Consider initializing the progress bar to reflect the starting state accurately.

+ m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
  progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetS3Keys[0]),
downloadS3ObjectsCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))
m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetS3Keys[0]),
downloadS3ObjectsCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))

Copy link
Owner Author

Choose a reason for hiding this comment

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

I Fix it. See 1fd8a10

Copy link
Contributor

Choose a reason for hiding this comment

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

@nao1215, I've reviewed the changes in commit 1fd8a10, and it looks like you've correctly implemented the initialization of the progress bar's index to accurately represent the starting state before any download starts. This should ensure that the progress display is accurate from the beginning of the operation. Great job on addressing the feedback!

  (\__/)
  (•ㅅ•) 
  / 　 づ
Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Fetch the changes made in the commit 1fd8a10 for the file ui/s3hub/list.go
git show 1fd8a10:ui/s3hub/list.go

Length of output: 19950

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

}
case "D":
if m.status == statusS3ObjectListed {
Expand All @@ -442,7 +458,12 @@ func (m *s3hubListS3ObjectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
}
m.sum = len(m.targetS3Keys) + 1
m.status = statusS3ObjectDeleting
return m, tea.Batch(m.spinner.Tick, deleteS3ObjectCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))

progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetS3Keys[0]),
deleteS3ObjectCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))
Comment on lines +465 to +470
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The progress bar's percentage calculation does not account for the initial state before any deletion starts, potentially leading to an inaccurate display. Consider initializing the progress bar to reflect the starting state accurately.

+ m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
  progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetS3Keys[0]),
deleteS3ObjectCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))
m.index = 0 // Initialize index to 0 to accurately represent the starting state of progress.
progressCmd := m.progress.SetPercent(float64(m.index) / float64(m.sum-1))
return m, tea.Batch(m.spinner.Tick,
progressCmd,
tea.Printf("%s %s", checkMark, m.targetS3Keys[0]),
deleteS3ObjectCmd(m.ctx, m.app, m.bucket, m.targetS3Keys[0]))

Copy link
Owner Author

Choose a reason for hiding this comment

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

I Fix it. See 1fd8a10

Copy link
Contributor

Choose a reason for hiding this comment

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

@nao1215, I've reviewed the changes in commit 1fd8a10a17eb59152e43982e6ba4f25f1bc7c8f3, and it looks like you've correctly implemented the initialization of the index to 0 before starting the progress bar for both bucket and S3 object operations. This should indeed ensure that the progress bar accurately represents the starting state of the operation. Great job on addressing the feedback!

(_/)
( •_•)
/>🎉

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Fetch the changes made in the specified commit and filter for the relevant file.
git show 1fd8a10a17eb59152e43982e6ba4f25f1bc7c8f3 -- ui/s3hub/list.go

Length of output: 1950

New Learnings Added 
User: nao1215
PR: nao1215/rainbow#52
File: ui/s3hub/list.go:465-470
Timestamp: 2024-02-03T08:44:09.840Z
Learning: The user has implemented the initialization of the index to `0` before starting the progress bar for both bucket and S3 object operations in commit `1fd8a10a17eb59152e43982e6ba4f25f1bc7c8f3`, ensuring accurate representation of the starting state.

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

}
case "enter":
if m.status == statusReturnToTop || m.status == statusDownloaded || m.status == statusS3ObjectDeleted {
Expand Down
Loading