-
Notifications
You must be signed in to change notification settings - Fork 10
Berickson/20260113 extension tab ids #254
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
Berickson/20260113 extension tab ids #254
Conversation
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c43d1da898
ℹ️ 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".
| t.95yb7msfl8ul | ||
| Return null if not a valid URL or tab param. | ||
| */ | ||
| var match = url.match(/[?&]tab=([^&]+)/i); |
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.
Strip URL fragment from tab_id extraction
The regex captures everything after tab= until the next &, but window.location.href often contains a fragment (e.g. #heading=h...) after the query string. In that case googledocs_tab_id_from_url() returns t.xxx#heading=h... instead of just the tab ID, so the tab_id logged from the content script will be polluted with fragment data and won’t match the real tab identifier. Consider parsing with URLSearchParams or stopping at # (e.g. [^&#]+) so the tab ID stays consistent when a document is opened via a heading link.
Useful? React with 👍 / 👎.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
fa87b6d to
dc98ec5
Compare
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c4ef3bcb2
ℹ️ 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".
| def _iter_leaf_commands(client): | ||
| for cmd in _iter_commands_from_client(client): | ||
| if not isinstance(cmd, dict): | ||
| continue | ||
|
|
||
| if cmd.get("ty") == "mlti": | ||
| for sub in cmd.get("mts") or []: | ||
| if isinstance(sub, dict): | ||
| yield sub | ||
| else: | ||
| yield cmd |
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.
Unwrap
nm commands when extracting tab titles
The tab-title extraction pipeline only flattens mlti commands in _iter_leaf_commands, but the same commit adds support for named tab commands (nm) in the reconstruct logic. If tab title updates (ucp/ac/mkch) arrive inside an nm wrapper, _extract_all_tab_titles will never see them, so tab_list won’t create or update tab entries. This means per-tab metadata can stay empty even when tabs exist and are renamed. Consider unwrapping nm (e.g., yield its nmc payload) alongside mlti so title commands nested under nm are processed.
Useful? React with 👍 / 👎.
No description provided.