Skip to content

Conversation

@rolandcrosby-columntax
Copy link
Contributor

Motivation

The "toggle block style" refactor action currently only works if there is a selection in the editor which covers a call node and its associated block. When there's no selection but the cursor is inside a block, it would be nice to be able to toggle the style of the block that the cursor is inside.

Implementation

  • In the CodeActions request: add a quick check using @document.locate_node to conditionally offer the "toggle block style" action if the cursor is inside a BlockNode
  • In the CodeActionResolve request: add a similar call to find the current block node when there's no selection. The actual rewrite also needs the CallNode associated with the block, and I couldn't figure out a good way to do this without adding a second RubyDocument.locate call. (When you call locate_node to get the BlockNode, you don't get any reference to the CallNode that the BlockNode is attached to. You also can't just look for a CallNode covering the cursor - that could return a CallNode that's actually inside the block that you want to toggle.)

Automated Tests

Added some expectation tests:

  • verify that the code action is offered when the cursor is inside a block and isn't offered when the cursor is outside a block
  • verify that the rewrite action toggles the expected block(s) depending on the cursor position

Manual Tests

  • Place your cursor outside a block and trigger code actions => none should be available.
  • Place your cursor inside a block and trigger code actions => "Refactor: Toggle block style" should be available, and should toggle the style of the block that the cursor is inside (and any nested blocks)

@rolandcrosby-columntax rolandcrosby-columntax requested a review from a team as a code owner November 10, 2025 16:48
@graphite-app
Copy link

graphite-app bot commented Nov 10, 2025

How to use the Graphite Merge Queue

Add the label graphite-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@rolandcrosby-columntax
Copy link
Contributor Author

I have signed the CLA!

@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Dec 15, 2025
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, this is great. Let's just ensure we're not traversing the AST multiple times for discovering code actions and we should be good to ship

@egiurleo egiurleo assigned vinistock and unassigned alexcrocha Jan 5, 2026
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@vinistock
Copy link
Member

Huh, I'm not sure why CI got stuck. @rolandcrosby-columntax could you please rebase to kick off CI again?

@rolandcrosby-columntax rolandcrosby-columntax force-pushed the toggle-block-without-selection branch from 7de9745 to b64ba68 Compare January 8, 2026 16:39
@rolandcrosby-columntax rolandcrosby-columntax force-pushed the toggle-block-without-selection branch from b64ba68 to fd0deb9 Compare January 8, 2026 16:42
@rolandcrosby-columntax
Copy link
Contributor Author

@vinistock Done. Thanks for reviewing, and let me know if you need anything else to get this merged!

@vinistock vinistock merged commit ab544c3 into Shopify:main Jan 8, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request server This pull request should be included in the server gem's release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants