Add a right-side gutter CodeMirror extension #2561
Conversation
WalkthroughThis pull request introduces a new entry to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/utils/code-mirror-gutter-rs.ts (2)
12-19: Remove Commented-Out CodeThe commented-out import statements are unnecessary and may clutter the codebase. Removing them can improve readability.
Consider deleting lines 12 to 19 to clean up the code.
258-263: Consider Adding Unit Tests foradvanceCursorFunctionThe
advanceCursorfunction is a critical part of the gutter implementation. Adding unit tests can help ensure its correctness and prevent future regressions.Would you like assistance in writing unit tests for this function?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 260-260: app/utils/code-mirror-gutter-rs.ts#L260
Added line #L260 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.prettierignore(1 hunks)app/components/code-mirror.ts(2 hunks)app/utils/code-mirror-collapse-unchanged-gutter.ts(3 hunks)app/utils/code-mirror-gutter-rs.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .prettierignore
🧰 Additional context used
🪛 GitHub Check: codecov/patch
app/utils/code-mirror-gutter-rs.ts
[warning] 96-96: app/utils/code-mirror-gutter-rs.ts#L96
Added line #L96 was not covered by tests
[warning] 170-170: app/utils/code-mirror-gutter-rs.ts#L170
Added line #L170 was not covered by tests
[warning] 205-205: app/utils/code-mirror-gutter-rs.ts#L205
Added line #L205 was not covered by tests
[warning] 207-207: app/utils/code-mirror-gutter-rs.ts#L207
Added line #L207 was not covered by tests
[warning] 220-223: app/utils/code-mirror-gutter-rs.ts#L220-L223
Added lines #L220 - L223 were not covered by tests
[warning] 225-225: app/utils/code-mirror-gutter-rs.ts#L225
Added line #L225 was not covered by tests
[warning] 227-228: app/utils/code-mirror-gutter-rs.ts#L227-L228
Added lines #L227 - L228 were not covered by tests
[warning] 231-232: app/utils/code-mirror-gutter-rs.ts#L231-L232
Added lines #L231 - L232 were not covered by tests
[warning] 235-236: app/utils/code-mirror-gutter-rs.ts#L235-L236
Added lines #L235 - L236 were not covered by tests
[warning] 247-247: app/utils/code-mirror-gutter-rs.ts#L247
Added line #L247 was not covered by tests
[warning] 260-260: app/utils/code-mirror-gutter-rs.ts#L260
Added line #L260 was not covered by tests
[warning] 294-294: app/utils/code-mirror-gutter-rs.ts#L294
Added line #L294 was not covered by tests
[warning] 300-300: app/utils/code-mirror-gutter-rs.ts#L300
Added line #L300 was not covered by tests
[warning] 326-327: app/utils/code-mirror-gutter-rs.ts#L326-L327
Added lines #L326 - L327 were not covered by tests
[warning] 329-331: app/utils/code-mirror-gutter-rs.ts#L329-L331
Added lines #L329 - L331 were not covered by tests
[warning] 333-333: app/utils/code-mirror-gutter-rs.ts#L333
Added line #L333 was not covered by tests
[warning] 335-335: app/utils/code-mirror-gutter-rs.ts#L335
Added line #L335 was not covered by tests
[warning] 341-343: app/utils/code-mirror-gutter-rs.ts#L341-L343
Added lines #L341 - L343 were not covered by tests
[warning] 351-351: app/utils/code-mirror-gutter-rs.ts#L351
Added line #L351 was not covered by tests
[warning] 426-426: app/utils/code-mirror-gutter-rs.ts#L426
Added line #L426 was not covered by tests
[warning] 447-449: app/utils/code-mirror-gutter-rs.ts#L447-L449
Added lines #L447 - L449 were not covered by tests
[warning] 452-452: app/utils/code-mirror-gutter-rs.ts#L452
Added line #L452 was not covered by tests
[warning] 459-459: app/utils/code-mirror-gutter-rs.ts#L459
Added line #L459 was not covered by tests
[warning] 461-461: app/utils/code-mirror-gutter-rs.ts#L461
Added line #L461 was not covered by tests
[warning] 463-463: app/utils/code-mirror-gutter-rs.ts#L463
Added line #L463 was not covered by tests
[warning] 467-467: app/utils/code-mirror-gutter-rs.ts#L467
Added line #L467 was not covered by tests
[warning] 473-473: app/utils/code-mirror-gutter-rs.ts#L473
Added line #L473 was not covered by tests
[warning] 476-476: app/utils/code-mirror-gutter-rs.ts#L476
Added line #L476 was not covered by tests
[warning] 479-480: app/utils/code-mirror-gutter-rs.ts#L479-L480
Added lines #L479 - L480 were not covered by tests
[warning] 483-483: app/utils/code-mirror-gutter-rs.ts#L483
Added line #L483 was not covered by tests
🪛 Biome (1.9.4)
app/utils/code-mirror-gutter-rs.ts
[error] 301-301: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 382-382: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (3)
app/utils/code-mirror-collapse-unchanged-gutter.ts (2)
47-62: Implementation ofCollapseUnchangedGutterMarkerRSLooks GoodThe new
CollapseUnchangedGutterMarkerRSclass correctly extendsGutterMarkerRSand implements the required methods. This enhances the functionality for right-side gutters.
74-80: Integration of Right-Side Gutter incollapseUnchangedGutterThe addition of the
gutterRSconfiguration integrates the right-side gutter functionality effectively. This is a valuable enhancement to the existing feature.app/components/code-mirror.ts (1)
70-71: Verify the Inclusion of Both Gutter Highlight ExtensionsIncluding both
highlightActiveLineGutter()andhighlightActiveLineGutterRS()may lead to overlapping functionality or visual inconsistencies. Ensure that both are necessary, or consider using only the right-side gutter version to avoid potential conflicts.
VasylMarchuk
force-pushed
the
code-mirror/right-side-gutter
branch
from
6f44d97 to
3419cf7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/utils/code-mirror-gutter-rs.ts (1)
5-6: 🛠️ Refactor suggestionAvoid Disabling TypeScript and ESLint Checks
Disabling TypeScript (
@ts-nocheck) and ESLint (/* eslint-disable */) may hide potential issues and reduce code quality. It's recommended to address the underlying errors instead of disabling the checks to maintain code reliability.Consider removing the disable comments and resolving any TypeScript and ESLint errors.
🧹 Nitpick comments (2)
app/utils/code-mirror-gutter-rs.ts (1)
11-18: Remove Commented-Out Code for Better ReadabilityThe commented-out import statements may clutter the code and reduce readability. If these imports are no longer needed, consider removing them to improve code clarity.
Apply this diff to remove the commented-out code:
-// import {combineConfig, MapMode, Facet, Extension, EditorState, -// RangeValue, RangeSet, RangeCursor} from "@codemirror/state" -// import {EditorView} from "./editorview" -// import {ViewPlugin, ViewUpdate} from "./extension" -// import {BlockType, WidgetType} from "./decoration" -// import {BlockInfo} from "./heightmap" -// import {Direction} from "./bidi"app/utils/code-mirror-collapse-unchanged-gutter.ts (1)
47-62: Reduce Code Duplication by Reusing Existing FunctionalityThe
CollapseUnchangedGutterMarkerRSclass duplicates code fromCollapseUnchangedGutterMarker. To adhere to the DRY (Don't Repeat Yourself) principle, consider refactoring to avoid duplication. You could extend the existing class or abstract common functionality into a shared base class.Example refactor by extending the existing class:
-import { gutter as gutterRS, GutterMarker as GutterMarkerRS } from 'codecrafters-frontend/utils/code-mirror-gutter-rs'; +import { gutter as gutterRS, GutterMarker } from 'codecrafters-frontend/utils/code-mirror-gutter-rs'; -export class CollapseUnchangedGutterMarkerRS extends GutterMarkerRS { +export class CollapseUnchangedGutterMarkerRS extends CollapseUnchangedGutterMarker { constructor(view: EditorView, widget: WidgetType, line: BlockInfo) { super(view, widget, line); } }This way,
CollapseUnchangedGutterMarkerRScan utilize the existing implementation while incorporating any necessary modifications.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/components/code-mirror.ts(2 hunks)app/utils/code-mirror-collapse-unchanged-gutter.ts(3 hunks)app/utils/code-mirror-gutter-rs.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/utils/code-mirror-gutter-rs.ts
[error] 300-300: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 381-381: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (3)
app/utils/code-mirror-gutter-rs.ts (2)
300-301: Avoid Assignments Within ExpressionsAssigning to variables within expressions can be confusing and may lead to unintended side effects. It's clearer to separate the assignment from the expression.
Apply this diff to improve code clarity:
-if (marker) (markers || (markers = [])).push(marker) +if (marker) { + if (!markers) markers = []; + markers.push(marker); +}🧰 Tools
🪛 Biome (1.9.4)
[error] 300-300: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
381-382: Avoid Assignments Within ExpressionsAssigning within a conditional expression can reduce code readability. Separating the assignment from the condition enhances clarity.
Apply this diff to improve code clarity:
-if (this.above != above) - this.dom.style.marginTop = (this.above = above) ? above + "px" : "" +if (this.above != above) { + this.above = above; + this.dom.style.marginTop = above ? above + "px" : ""; +}🧰 Tools
🪛 Biome (1.9.4)
[error] 381-381: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
app/components/code-mirror.ts (1)
70-71: Verify Compatibility of Multiple Active Line Gutter HighlightsIncluding both
highlightActiveLineGutter()andhighlightActiveLineGutterRS()may cause conflicts or unintended behavior, as they might apply overlapping styles or manipulate the same DOM elements.Please test the editor to ensure that both gutter highlighting extensions work together without issues, and that the active line gutter is highlighted correctly. Confirm that there are no visual glitches or performance impacts.
|
Just wondering now if it's possible to completely exclude a file from codecov, so it doesn't ruin the project score |
Related to #1231
Brief
This adds a new right-side gutter extension for CodeMirror, extracted from the line comments PR, to add right-side gutter functionality required for the line comments extension, while shrinking the line comments PR.
Details
gutter.tsfromcodemirror/[email protected]intoapp/utils/code-mirror-gutter-rs.tsapp/utils/code-mirror-gutter-rs.tsto make it right-sidecollapseUnchangedGutterapp/utils/code-mirror-gutter-rs.tsto.prettierignoreMore details
The gutter extension was cloned as is from
@codemirror/[email protected], as suggested in this discussion, with the required modifications applied to it in a follow-up commit, including all necessary linting ignore rules.The idea is to have a commit with the original cloned file, and another commit with modifications, so that modifications can be re-applied in the future, if we need to update the extension to the latest
@codemirror/viewversion.The alternative would be to fork the
@codemirror/viewrepository, and apply changes there, and use the fork, but setting the fork up and maintaining it in the future seems like too much effort, for now.Checklist
[percy]in the message to trigger)Summary by CodeRabbit
New Features
Refactor
Chores