[PB Extension] Add command to open lexicon selector - #2684
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe extension adds ChangesLexicon selector command
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Merge Risk: ⚪ Minimal · up to The selector command preserves the intended result and registration behavior; no merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit taps the selector bright Comment |
Register `lexicon.openSelector(projectId)`, a thin wrapper on `ProjectManager.openSelector()` that resolves the project manager from a project id. It reports whether the selector opened, not whether a lexicon was chosen; the chosen lexicon lands in `lexicon.lexiconCode`, which a caller can watch. Adds no policy of its own about when relinking a project is appropriate, and no UX a user of this extension on its own can see. Fixes #2683 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ecd5d67 to
32df501
Compare
myieye
left a comment
There was a problem hiding this comment.
This looks ok to me, but I'm puzzled as to why it's being added.
It's essentially identical to the command that's marked for future deletion lexicon.changeLexicon.
Should it actually block somehow if a lexicon is already selected?
Should it be coupled with a second command for explicitly, intentionally clearing the current lexicon? Or should a flag be added to lexicon.openSelector to make overriding a current lexicon explicitly allowed?
Selection is sticky, so lexicon.openSelector now fails with an error when the project already has a lexicon. Clearing lexicon.lexiconCode first is the supported way to relink. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fair questions! A substantial difference from The desired route (at least for now) to clear the lexicon is to delete it via the project settings. We can add override later if a use-case demands it. |
Register
lexicon.openSelector(projectId)for the interlinearizer (sillsdev/interlinearizer-extension#44). It is a thin wrapper onProjectManager.openSelector()that resolves the project manager from a project id.It refuses a project that already has a lexicon. Selection is sticky: to relink, clear the project's
lexicon.lexiconCodesetting first.It reports whether the selector opened, not whether a lexicon was chosen. The chosen lexicon lands in
lexicon.lexiconCode, which a caller can watch.No UX change a user of this extension on its own can see.
Fixes #2683
Drafted by Claude Opus 5
Devin review: https://app.devin.ai/review/sillsdev/languageforge-lexbox/pull/2684