fix: Fix "Mark decorations may not be empty" issue #325
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce stricter validation when creating decorations for code ranges, ensuring that only non-empty ranges (where the start is strictly less than the end) are decorated. This logic is applied across range highlighting, symbol hover targets, and search result highlights to prevent zero-length or invalid decorations from being generated. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/web/src/lib/extensions/searchResultHighlightExtension.ts (1)
36-42: Effective range validation with functional approach.The implementation correctly prevents invalid decorations by returning
undefinedfor zero-length ranges and filtering them out. The functional programming approach is clean and readable.Consider consistency: while this map-filter pattern works well, the other files use conditional array pushing. Both approaches are valid, but maintaining consistency across similar validation logic could improve code maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts(1 hunks)packages/web/src/lib/extensions/searchResultHighlightExtension.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (1)
67-69: LGTM! Proper range validation prevents empty decorations.The addition of
node.from < node.tocheck correctly prevents zero-length ranges from being decorated, which would cause the "Mark decorations may not be empty" error. This is a solid defensive programming approach.packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts (1)
23-28: Good implementation of range validation.The conditional decoration creation properly handles edge cases where
from >= to, preventing the "Mark decorations may not be empty" error. The approach of using an empty array and conditionally adding decorations is clean and maintainable.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
11-11: Fix typo and grammar in changelog entryThe verb “display” should be “displayed,” and it reads more naturally as “on the login page” rather than “in.”
- Fixed issue where new oauth providers weren't being display in the login page. [commit](https://github.com/sourcebot-dev/sourcebot/commit/a2e06266dbe5e5ad4c2c3f730c73d64edecedcf7) + Fixed issue where new OAuth providers weren't being displayed on the login page. [commit](https://github.com/sourcebot-dev/sourcebot/commit/a2e06266dbe5e5ad4c2c3f730c73d64edecedcf7)🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...where new oauth providers weren't being display in the login page. [commit](https://git...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~11-~11: The preposition “on” seems more likely in this position than the preposition “in”.
Context: ...w oauth providers weren't being display in the login page. [commit](https://github...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_ON)
12-12: Standardize formatting for error message entryHyphenate “client-side” and use code formatting for the error string to improve readability.
- Fixed client side "mark decorations may not be empty" error when viewing certain files. [#325](https://github.com/sourcebot-dev/sourcebot/pull/325) + Fixed client-side error `mark decorations may not be empty` when viewing certain files. [#325](https://github.com/sourcebot-dev/sourcebot/pull/325)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~11-~11: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...where new oauth providers weren't being display in the login page. [commit](https://git...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~11-~11: The preposition “on” seems more likely in this position than the preposition “in”.
Context: ...w oauth providers weren't being display in the login page. [commit](https://github...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_ON)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
Was hitting a "Mark decorations may not be empty" client side error for certain files. Fix is to do a range check before applying the decoration.
Summary by CodeRabbit