Remove deprecated expr_string module #287
Conversation
WalkthroughRemoved the ExprString feature and its public exposure: deleted Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Stringify as stringify(value)
Caller->>Stringify: value
alt nullish / function / symbol
Stringify-->>Caller: "v:null"/"function()"/"symbol()"
else boolean / number
Stringify-->>Caller: Vim literal (true/false/number)
else string
Note right of Stringify #ffeead: ExprString branch removed
Stringify-->>Caller: Single-quoted Vim string (escape single-quotes)
else array
Stringify-->>Caller: [ ... ] serialized
else object / toJSON
Stringify-->>Caller: { ... } serialized
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (8)
💤 Files with no reviewable changes (7)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
helper/keymap.ts (1)
55-56: Typo in user-facing docs: “remaped” → “remapped”.Minor, but visible in published docs/examples.
- * // Send remaped keys. + * // Send remapped keys.
🧹 Nitpick comments (1)
helper/keymap.ts (1)
22-26: Normalize remap when a Keys object is supplied (JS callers safety).Today's code returns the object as-is. If a JS caller passes
{ keys: "...", remap: undefined }, it will behave as false at runtime, but normalizing makes the intent explicit and future-proof.Apply this diff:
function toKeys(keys: KeysSpecifier): Keys { if (isString(keys) || isRawString(keys)) { return { keys, remap: false }; } - return keys; + // Normalize to ensure `remap` is a strict boolean for untyped JS callers + return { keys: keys.keys, remap: !!keys.remap }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
deno.jsonc(0 hunks)eval/stringify.ts(0 hunks)eval/stringify_test.ts(0 hunks)helper/expr_string.ts(0 hunks)helper/expr_string_test.ts(0 hunks)helper/keymap.ts(2 hunks)helper/keymap_test.ts(0 hunks)helper/mod.ts(0 hunks)
💤 Files with no reviewable changes (7)
- helper/keymap_test.ts
- deno.jsonc
- helper/mod.ts
- eval/stringify.ts
- helper/expr_string_test.ts
- eval/stringify_test.ts
- helper/expr_string.ts
🧰 Additional context used
🧬 Code graph analysis (1)
helper/keymap.ts (1)
eval/string.ts (2)
RawString(38-38)isRawString(126-132)
🔇 Additional comments (2)
helper/keymap.ts (2)
11-11: Dropped ExprString from public Keys type — aligned with the deprecation removal.Type surface now matches the project direction (string | RawString only). Good cleanup.
10-13: Confirm and document breaking change forKeysexportI ran the provided
rgsearch across the codebase and found no remaining internal references toExprString,exprQuote,isExprString,useExprString, or thehelper/expr_stringmodule.However, since you’ve narrowed a public export (
Keysnow only acceptsstring | RawString), this is a breaking change. Please:
- Verify external impact: Confirm that no downstream or external consumers still pass
ExprString(or useexprQuote) to anyhelper/keymapAPIs.- Changelog & version bump: Document this change in your changelog/release notes and bump the package version accordingly.
- Migration guidance: Add a short migration note, e.g.:
Replace
ExprString/exprQuotewithRawStringwhen callinghelper/keymapAPIs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #287 +/- ##
==========================================
- Coverage 85.84% 85.83% -0.02%
==========================================
Files 65 64 -1
Lines 3468 3296 -172
Branches 302 277 -25
==========================================
- Hits 2977 2829 -148
+ Misses 489 465 -24
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The expr_string module and its associated types (ExprString, exprQuote, isExprString, useExprString) have been deprecated in favor of the eval module's rawString functionality. This commit removes all deprecated code and updates dependent files to use the new API. Changes: - Remove helper/expr_string.ts and helper/expr_string_test.ts - Update helper/keymap.ts to remove ExprString type support - Update eval/stringify.ts to remove isExprString checks - Remove related test cases from affected test files - Remove exports from helper/mod.ts and deno.jsonc 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
lambdalisue
force-pushed
the
remove-deprecated-expr-string
branch
from
6089d9a to
48ab793
Compare
Summary
helper/expr_stringmodule and all its exports (ExprString,exprQuote,isExprString,useExprString)Background
The
expr_stringmodule was deprecated in favor of theevalmodule'srawStringfunctionality. This PR completes the deprecation by removing all deprecated code.Changes Made
Removed files:
helper/expr_string.ts(285 lines)helper/expr_string_test.ts(493 lines)Updated files:
helper/keymap.ts: RemovedExprStringtype support, now only supportsstring | RawStringhelper/keymap_test.ts: Removed test cases usingexprQuoteeval/stringify.ts: RemovedisExprStringcheckseval/stringify_test.ts: Removed test cases forExprStringserializationhelper/mod.ts: Removed export of expr_string moduledeno.jsonc: Removed export entries for expr_string pathsTest Plan
stringandRawStringtypesExprStringsupport🤖 Generated with Claude Code
Summary by CodeRabbit