feat: Use pager for showing logs #414
Conversation
|
Hi @msvticket. Thanks for your PR. I'm waiting for a jenkins-x-plugins or todo member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository. |
|
Kudos, SonarCloud Quality Gate passed!
|
|
/assign @ankitm123 |
|
/ok-to-test |
| @@ -6,6 +6,7 @@ require ( | |||
| github.com/charmbracelet/bubbletea v0.13.1 | |||
| github.com/cpuguy83/go-md2man v1.0.10 | |||
| github.com/fatih/color v1.10.0 | |||
| github.com/gerow/pager v0.0.0-20190420205801-6d4a2327822f | |||
There was a problem hiding this comment.
Is there an alternative to this package? It seems the last modification was done in April 2019, is it maintained?
There was a problem hiding this comment.
The only alternative I've found was updated even longer ago: github.com/sean-/pager
Well, there is a fork of that with a couple of later commits that might be an alternative: github.com/peloy/pager
But the drawback is that for that library a separate check is needed to know if a terminal is attached.
No, I don't get the impression it is maintained.
There was a problem hiding this comment.
I will take a look at the source code of the pager package tonight (after work), and then get back to this PR. I think we can merge this tonight with gerow/pager, but may be long term we can add similar functionality to jx-helper (since u mentioned there are no go pager packages which are maintained) to ensure this part is maintained. wdyt?
There was a problem hiding this comment.
Sure. Also the functionality is isolated and while convenient it's not crucial. So in the worst case it can simply be removed.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ankitm123 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
When showing long logs the terminal buffer might not be enough and using a pager would be practical. But to pipe a log to pager you need to add enough filters to get only the required log and this can be cumbersome. With this fix a pager is used if in a terminal, not in batch mode and if a pager is available.
The reason I don't implement the same feature for grid is that executing a sub command is interfering with bubbletea (See charmbracelet/bubbletea#171)