Skip to content

Conversation

@clipperhouse
Copy link
Owner

Encode regional indicators into the trie instead of if’s.

Also, asciiWidth tables are too much complication, just make that a conditional.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the handling of Regional Indicator characters by encoding them into the trie data structure instead of using runtime if-statement checks. It also simplifies ASCII width calculation by replacing lookup tables with a conditional function.

Key changes:

  • Adds a new _Regional_Indicator property type to the trie
  • Removes the isRIPrefix() helper and associated runtime checks
  • Replaces asciiWidths and asciiProperties lookup tables with an asciiWidth() function

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
width.go Removes isRIPrefix() function and regional indicator byte-checking logic; adds property-based regional indicator handling in lookupProperties(); replaces asciiWidths table lookups with asciiWidth() function calls
tables.go Replaces asciiWidths and asciiProperties lookup tables with a simple asciiWidth() function; extends propertyWidths array to include _Regional_Indicator
trie.go Adds _Regional_Indicator constant; updates generated trie data to encode regional indicators as property value 0x0005
width_test.go Adds test case for single Regional Indicator character
tables_test.go Adds new test file with comprehensive tests for asciiWidth() function
internal/gen/unicode.go Updates Unicode data parser to identify and encode Regional Indicator characters in the trie

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@clipperhouse clipperhouse force-pushed the clipperhouse/regional-indicator branch from cdf3b26 to 2748a1a Compare November 29, 2025 18:19
@clipperhouse clipperhouse merged commit 09314c6 into main Nov 29, 2025
10 checks passed
@clipperhouse clipperhouse deleted the clipperhouse/regional-indicator branch November 29, 2025 18:24
clipperhouse added a commit that referenced this pull request Nov 29, 2025
@clipperhouse
Copy link
Owner Author

See e4aab45

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.

2 participants