Repository navigation
Support scrolling in vscode notebooks - #3380
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13b415a281
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| at: RevealLineAt, | ||
| ): Promise<void> { | ||
| if (isDocumentNotebookCell(editor.vscodeEditor.document)) { | ||
| revealNotebookCell(editor, at); |
There was a problem hiding this comment.
When the target is inside a notebook cell taller than the viewport, this branch discards lineNumber and aligns the entire cell instead. Since Scroll.getLineNumber computes the specific line occupied by the target, crown/center on a token near the bottom of a long cell can leave that token off-screen or at the wrong position; pass the line through and reveal the position within the cell rather than only the cell range.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is not supported by the vscode notebook api as far as I can tell
| case RevealLineAt.bottom: | ||
| throw new Error( | ||
| "VS Code's notebook API doesn't support bottom alignment.", | ||
| ); |
There was a problem hiding this comment.
Whenever scrollToBottom is invoked on a notebook target, this branch throws instead of completing the existing action, so users receive an error for every such command. The lack of a direct notebook reveal enum should be handled with a fallback or approximation rather than making the action fail outright; any unavoidable notebook-specific limitation also needs to be documented.
AGENTS.md reference: AGENTS.md:L32-L32
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
There is no way to make a fall back for this as far as I know
github-actions
Bot
added
lines/200-499
1 participant
Fixes #3379
Note that bottom alignment is not supported in the vscode notebook api, but crown and center works. There's also a limitation that we can't scroll to a specific line in a notebook cell we can only scroll the notebook to a specific cell. These are all limitations in the vscode notebook api and not something I think we can work around.