Skip to content

Clean up DocumentMessageHandler by breaking out several lengthy handlers into helper functions#3568

Merged
Keavon merged 1 commit into
GraphiteEditor:masterfrom
theaniketgiri:issue/2740-code-quality-refactors
Feb 17, 2026
Merged

Clean up DocumentMessageHandler by breaking out several lengthy handlers into helper functions#3568
Keavon merged 1 commit into
GraphiteEditor:masterfrom
theaniketgiri:issue/2740-code-quality-refactors

Conversation

@theaniketgiri

@theaniketgiri theaniketgiri commented Jan 2, 2026

Copy link
Copy Markdown
Contributor

Partly closes #2740

This PR addresses the "Match statements" section of the tracking issue by extracting three large match arms from process_message() into dedicated helper methods.

Changes

Extracted the following helper methods in DocumentMessageHandler:

Method Lines Extracted Purpose
handle_group_selected_layers ~50 lines Handles layer grouping in both artboard and non-artboard workflows
handle_move_selected_layers_to ~85 lines Handles layer movement with proper transform preservation
handle_nudge_selected_layers ~75 lines Handles keyboard nudging with optional resize mode

Benefits

  • Improved readability: The main match statement is now ~210 lines shorter
  • Better maintainability: Each operation is now self-contained in its own method
  • Easier testing: Helper methods can potentially be unit tested independently
  • Clearer intent: Method names describe what the code does

Before/After

Before:

DocumentMessage::GroupSelectedLayers { group_folder_type } => {
    // ~50 lines of inline code
}

After:

DocumentMessage::GroupSelectedLayers { group_folder_type } => {
    self.handle_group_selected_layers(group_folder_type, responses);
}

Testing

  • Verified compilation with cargo check
  • Logic and behavior remain unchanged - this is a pure refactor
  • Existing tests pass (note: SIGSEGV in tests is a pre-existing issue on master)

@timon-schelling timon-schelling marked this pull request as draft January 2, 2026 14:25
@timon-schelling

Copy link
Copy Markdown
Member

Why the new workflow? That seems unrelated.

Converted to draft because this not ready for review.

@theaniketgiri theaniketgiri force-pushed the issue/2740-code-quality-refactors branch from 50bf1ad to dd65498 Compare January 2, 2026 14:31
@theaniketgiri

Copy link
Copy Markdown
Contributor Author

Why the new workflow? That seems unrelated.

Converted to draft because this not ready for review.

Hi @timon-schelling, thanks for the review!

You're absolutely right - both the workflow file and pnpm-lock were accidentally included. They were from a different branch I was working on and got mixed in when I created this PR.

I've fixed this now:

Rebased onto master
Removed the unrelated workflow file and pnpm-lock
The PR now only contains the refactoring changes

@theaniketgiri theaniketgiri marked this pull request as ready for review January 2, 2026 14:37
@Keavon Keavon force-pushed the issue/2740-code-quality-refactors branch from dd65498 to e318a1f Compare February 17, 2026 21:38
@Keavon Keavon changed the title Issue/2740 code quality refactors Clean up DocumentMessageHandler by breaking out several lengthy handlers into helper functions Feb 17, 2026
@Keavon Keavon enabled auto-merge (squash) February 17, 2026 21:43
Part of GraphiteEditor#2740 - Code quality refactors.

This commit addresses the 'Match statements' section of the tracking
issue by extracting three large match arms from process_message() into
dedicated helper methods:

- handle_group_selected_layers: Handles layer grouping in both artboard
  and non-artboard workflows (~50 lines extracted)
- handle_move_selected_layers_to: Handles layer movement with proper
  transform preservation (~85 lines extracted)
- handle_nudge_selected_layers: Handles keyboard nudging with optional
  resize mode (~75 lines extracted)

The main match statement in DocumentMessageHandler is now ~210 lines
shorter and more readable. Logic and behavior remain unchanged.
@Keavon Keavon force-pushed the issue/2740-code-quality-refactors branch from e318a1f to 2963689 Compare February 17, 2026 21:48
@Keavon Keavon merged commit f801ed7 into GraphiteEditor:master Feb 17, 2026
3 checks passed
@Keavon

Keavon commented Jun 13, 2026

Copy link
Copy Markdown
Member

Hi @theaniketgiri, thanks again for this code contribution to the project! We're still hoping you will respond to the request to relicense this code. Please see #4208 ASAP, thank you!

@Keavon

Keavon commented Jun 15, 2026

Copy link
Copy Markdown
Member

Hi @theaniketgiri, we're grateful of your past code contribution to Graphite, but unless we hear from you by the end of this weekend with agreement about relicensing your code to include the MIT license, we will begin steps to remove your code contributions so that you will no longer be a Graphite code contributor. We'd much rather avoid this outcome, so please respond now.

To agree, simply log into your GitHub account, visit #4208, and copy-paste this into a comment:

"I license my past and future contributions to Graphite under the dual MIT/Apache-2.0 license, allowing licensees to choose either at their option."

If you're hesitant because you disagree or have questions, feel free to respond with another message; we'll do our best to answer your questions or concerns. More explanation is at the top of the linked thread if you're confused. So far, 164 contributors have agreed and you are among the 11% remaining whom we are still hoping to hear from before we begin removing your code from the project starting this Monday. (If you only see this later than Monday, your future belated response will still be appreciated.)

Thank you for your cooperation and support of open source, and we hope you'll help us keep your valued contributions in-tact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracking Issue: Code quality refactors

3 participants