8000 Refactor alias handling: simplify detection logic and reduce allocations by Copilot · Pull Request #23 · jackielii/skhd.zig · GitHub
[go: up one dir, main page]
Skip to content

Refactor alias handling: simplify detection logic and reduce allocations #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
Copilot wants to merge 2 commits into
base: claude/plan-key-alias-support-01Q8Q1qANjKGsL4x1iy4JpfJ
Choose a base branch
from
Draft

Refactor alias handling: simplify detection logic and reduce allocations #23

Copilot wants to merge 2 commits into from

Conversation

Copy link
Copilot AI commented Nov 19, 2025 •
edited
Loading

Restructures alias handling in parse_hotkey to address code review feedback on #21 requesting better high-level structure.

Changes

  • Forward-compatible standalone detection: Replace exhaustive token enumeration (Token_Command | Token_Forward | Token_BeginList | ...) with next_token == Token_Dash || next_token == Token_Plus. Alias is standalone unless followed by - or +.

  • Single visited list: Create once per hotkey, reuse with clearRetainingCapacity() instead of creating/destroying 3+ times per parse.

  • Clearer semantics: Rename is_standalone_keysym → key_parsed to reflect what it actually tracks (key already determined, not alias type).

Before/After

// Before: brittle, checks all possible action tokens
if (next_is_command or next_is_forward or next_is_begin_list or 
    next_is_unbound or next_is_arrow or next_is_activate or self.peek() == null) {
    var visited = std.ArrayList([]const u8).init(self.allocator);  // allocation #1
    defer visited.deinit();
    // ...
}

// After: forward-compatible, single allocation
var visited = std.ArrayList([]const u8).init(self.allocator);  // once
defer visited.deinit();

const is_modifier_use = if (next_token) |tok| 
    tok.type == .Token_Dash or tok.type == .Token_Plus 
else false;

Net: -5 lines, clearer logic, fewer allocations.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI self-assigned this Nov 19, 2025
Copilot AI mentioned this pull request Nov 19, 2025
Draft
@jackielii
Restructure alias handling logic for better clarity and maintainability
7b9eb48 
- Simplify standalone keysym detection: use forward-compatible check (is next token - or +?)
- Create single visited list for entire hotkey parsing instead of multiple times
- Rename is_standalone_keysym to key_parsed for clearer semantics
- Reduce code duplication and complexity

Co-authored-by: jackielii <360983+jackielii@users.noreply.github.com>
Copy link
Author
Copilot AI commented Nov 19, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • canonical-bos01.cdn.snapcraftcontent.com
    • Triggering command: /usr/lib/snapd/snapd (dns block)
  • ziglang.org
    • Triggering command: wget -q REDACTED (dns block)
    • Triggering command: curl -L -o zig.tar.xz REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Restructure PR for key alias support based on review feedback Refactor alias handling: simplify detection logic and reduce allocations Nov 19, 2025
Copilot AI requested a review from jackielii November 19, 2025 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jackielii jackielii Awaiting requested review from jackielii

Assignees

@jackielii jackielii

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jackielii
0