-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
livecheck: progress bar for JSON output #8577
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
livecheck: progress bar for JSON output #8577
Conversation
I was going to ask how long a livecheck run usually takes, but I decided to try it myself:
And it's still going. I can see why this might be good to have! |
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.
This is useful for when you're doing a run where you're not redirecting the output to a file and does deal with the issue where we have to wait in silence until livecheck spits out all the JSON at the end. I added a suggestion that provides a different progress bar format which may be a little more pleasant in some respects.
Unfortunately, the progress bar doesn't seem to be compatible with redirecting the output to a file. I do something like brew livecheck --json --tap homebrew/core > some_directory/filename.json
very frequently and this ends up producing a file that isn't valid JSON since the progress bar output appears at the beginning of the file.
One way of dealing with this would be to add a CLI option like --output-file
(or something like that), where livecheck would write the JSON (making sure we can create the file at the destination before doing the work, of course). I imagine we would only allow this option when the --json
flag is also present (until/unless someone comes along asking to do this for the normal output). This would allow us to have the best of both worlds, where we could have the progress bar while also outputting JSON to a file. This particular setup is exactly what I'm looking for.
The other issue is that the progress bar output would also appear in the output if you were to run the command in a script and capture the output (as happens in the utils/livecheck_formula.rb
file that brew bump
uses), so this would also result in invalid JSON. I added a suggestion of one potential way to address this but we may end up having to put the progress bar output behind a flag (e.g., --progress
) to make it opt-in rather than opt-out. I imagine users would expect the --json
flag to only output JSON under any circumstances and this breaks that contract.
With the latest push:
I think I've covered most (if not all) changes made. Let me know if anything else should be changed or improved. Thanks! |
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 can only comment on the changes within livecheck (and I'll rely on someone else to say whether the other parts are correct) but I think this should be good from my perspective after these final comments are resolved.
I did a run with the progress bar and --output-file=
and it's quite nice 👍
This isn't strictly needed given the |
Now that we have a solution to the redirection problem by writing the progress bar to |
I personally think there would be no need for |
Agreed. I don't think we have any other commands that take it but many that are designed to work differently if |
With the current However the problems are:
I tried adding an Any thoughts on how we should move forward? To me, it seems like we should revert to the |
The general consensus is that we shouldn't have an
I think how we proceed here depends on any |
I'll go ahead with option 3 and get the |
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.
Fine with me once @samford is happy. Thanks @nandahkrishna!
I tested this branch locally by rebasing on the latest main branch and making some I think this should be good after the related changes have been incorporated into this PR. |
Co-authored-by: Sam Ford <1584702+samford@users.noreply.github.com> Co-authored-by: Dustin Rodrigues <dust.rod@gmail.com>
@samford: I've rebased and incorporated the changes, let me know if this is good to merge, thanks. |
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.
Looks good to me now. Thanks, Nanda!
brew style
with your changes locally?brew tests
with your changes locally?This PR adds a progress bar for livecheck's JSON output. We often use the
--json
flag when we have too many formulae to check and would like to redirect the livecheck output to a file, maybe to use later in a script or to compare livecheck outputs after making changes to livecheckables.Currently, no feedback or progress information is provided, and we're left wondering how long it's going to take until livecheck is done. The addition of the progress bar keeps us updated on the status and helps us anticipate how much longer the process would take. I used the
ruby-progressbar
gem as I was unable to find any other command or module which had a progress bar (curl
uses its own--progress-bar
option), so do let me know if I should be doing it some other way.This progress bar is temporary – it stays on-screen until livecheck is done preparing the JSON output, following which it is erased (for a TTY stdout). This 'feature' required the addition of some additional
SPECIAL_CODES
to theTty
module (to clear a line and move up a line, I added a few more to make sure we have a 'complete' set).I think I've covered all the details here. Looking forward to your comments/suggestions!
CC @samford