Fix rename cursor boundary for LiquidDoc param names#1124
Conversation
When the cursor is at the first character of a LiquidDoc @param name, rename was not triggering because the node boundary check used the LiquidDocParamNode position (which includes the @ prefix) instead of the paramName position. Now we detect LiquidDocParamNode and use its paramName sub-node for boundary calculations. Closes #936 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
packages/theme-language-server-common/src/rename/providers/LiquidVariableRenameProvider.spec.ts
Outdated
Show resolved
Hide resolved
packages/theme-language-server-common/src/rename/providers/LiquidVariableRenameProvider.ts
Outdated
Show resolved
Hide resolved
packages/theme-language-server-common/src/rename/providers/LiquidVariableRenameProvider.spec.ts
Outdated
Show resolved
Hide resolved
|
Did 🎩 , worked great. |
| if (node.type === NodeTypes.LiquidDocParamNode) { | ||
| const cursorOffset = textDocument.offsetAt(params.position); | ||
| const nameEnd = node.paramName.position.start + node.paramName.value.length; | ||
| if (cursorOffset < node.paramName.position.start || cursorOffset > nameEnd) return null; | ||
| } |
There was a problem hiding this comment.
I wonder if we should actually change the logic for findCurrentNode instead... should probably check how hover behave. If hover is also bugged we might want to revisit.
There was a problem hiding this comment.
Oh we don't have a hover for that one.
There was a problem hiding this comment.
Aight after some research the approach in the PR is better.
- Hover is silly in the doc itself, since the doc is right after it.
- Completion doesn't make sense, since we're defining it.
Drawbacks of changing isCovered in visitor.ts:
- It would require changes to how
TextNodecalculates isCovered which would create an ambiguity for things like<tag>TextNode</tag>. Right now it's resolved by not matching on start so the>matches on the HtmlElement.
aswamy
left a comment
aswamy
left a comment
There was a problem hiding this comment.
I noticed an existing bug - but since yours don't cause it or make it worse, we can do a quick follow-up.
Existing bug: If you have @param {image} image and you rename image, it renames the type and the variable name. We need a patch for that.
Other than that, tophat went well ✅
…the same text Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Folded that fix in :) TY for reporting! |
2 participants
Summary
Fixes #936
Rename does not trigger when the cursor is at the first character of a
@paramname in a LiquidDoc block.🤖 Generated with Claude Code