-
Notifications
You must be signed in to change notification settings - Fork 738
feat(cwl): Support autoscrolling live tail session's visible editors #5857
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
Conversation
|
This pull request modifies code in src/ but no tests were added/updated. Confirm whether tests should be added or ensure the PR description explains why tests are not required. |
|
This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions. |
packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts
Outdated
Show resolved
Hide resolved
| //Auto scroll visible LiveTail session editors if the end-of-file is in view. | ||
| //This allows for newly added log events to stay in view. | ||
| function getTextEditorsToScroll(document: vscode.TextDocument): vscode.TextEditor[] { | ||
| return vscode.window.visibleTextEditors.filter((editor) => { |
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.
If someone has a two visible editors, one on the left and one on the right like:
____________________________________________________
| | |
| first editor | second editor |
| | |
____________________________________________________
does that mean both of them will scroll?
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.
Yep! they'd both be considered visible and able to be scrolled if at EOF. If one isn't at EOF, and the other is, then only one would be scrolled.
| //This allows for newly added log events to stay in view. | ||
| function getTextEditorsToScroll(document: vscode.TextDocument): vscode.TextEditor[] { | ||
| return vscode.window.visibleTextEditors.filter((editor) => { | ||
| const isEditorForSession = editor.document === document |
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 this is only used once, should you just inline this on line 111
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 could do that!
The reason I wrote this as a function was not so much about reusability, but more about readability. The code going into handleSessionStream is growing in size and complexity. I worry that as we do more per stream update that it will become less clear what "actions" we take. Having this defined as a function makes it easier to see that per stream update that we need to 'getTextEditorsToScroll`, then update the document, then scroll it. While abstracting away those implementations.
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.
Oh yeah, I thought that make sense. I was more just commenting on doing:
if (editor.document !== document) {
...
}
vs
const isEditorForSession = editor.document === document
if (!isEditorForSession) {
}
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.
Oh haha I misunderstood 😅 I'll change this
…ws#5857) ## Problem New log events in a LiveTail session are added to the end of a TextDocument. Session updates can contain multiple LogEvents, leading to the text document growing quickly past the user's view. requiring them to constantly be scrolling the document themselves. ## Solution To maintain parity with CWL console experience (and the classic `tail -f` experience), we want to auto scroll the customer's LiveTail session's visible editors to the bottom. This will only happen IF the end of file is already in view. Meaning, if a User scrolls UP (causing the end of file to not be in view), auto scrolling will not enable. However, if they scroll back down to EOF themselves, it will re-enable. Simply, if the end of file is in view, when new logEvents are added to the document, the editor will be autoscrolled back to the end of the file.
Problem
New log events in a LiveTail session are added to the end of a TextDocument. Session updates can contain multiple LogEvents, leading to the text document growing quickly past the user's view. requiring them to constantly be scrolling the document themselves.
Solution
To maintain parity with CWL console experience (and the classic
tail -fexperience), we want to auto scroll the customer's LiveTail session's visible editors to the bottom.This will only happen IF the end of file is already in view. Meaning, if a User scrolls UP (causing the end of file to not be in view), auto scrolling will not enable. However, if they scroll back down to EOF themselves, it will re-enable.
Simply, if the end of file is in view, when new logEvents are added to the document, the editor will be autoscrolled back to the end of the file.
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.