[codex] Add bulk page import - #6
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Free Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| entry.startsWith("#/") || | ||
| entry.startsWith("/#/"); | ||
|
|
||
| export const createQuickSwitcherSettingsComponent = ({ |
There was a problem hiding this comment.
🚩 Component state initialized from closure-captured initialBookmarks won't reflect external updates
The QuickSwitcherSettings component initializes bookmarks via useState(initialBookmarks) where initialBookmarks is captured by the createQuickSwitcherSettingsComponent closure (src/components/QuickSwitcherSettings.tsx:55). If bookmarks are modified externally (e.g. via keyboard shortcut registration in src/quickSwitcher.tsx:242-247), this component won't see those changes until re-mounted. This is a pre-existing pattern not introduced by this PR, but the bulk import feature makes it more likely that users open the settings dialog multiple times in one session, increasing the chance of stale state.
Was this helpful? React with 👍 or 👎 to provide feedback.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
Blocking feedback
- Bulk import can throw on malformed URL-encoded input and abort the whole add flow —
src/components/QuickSwitcherSettings.tsx#L240
If you want Charlie to apply this fix, reply with item numbers (for example: please fix 1).
|
|
||
| entries.forEach((entry) => { | ||
| const pageUid = isPageUrlInput({ entry }) | ||
| ? parsePageUidFromUrl({ url: entry }) |
There was a problem hiding this comment.
parsePageUidFromUrl can throw a URIError on malformed percent-encoded URLs (for example https://roamresearch.com/#/app/graph/page/%). Because this call is unguarded, one bad line aborts addBulkPages entirely instead of skipping that entry.
Suggested fix: make URL parsing fail-safe here (or in parsePageUidFromUrl) by catching decode/parsing errors and treating invalid lines as skipped entries.
1 participant
Summary
Validation