Skip to content

fix(code-editor): trigger completion for unicode identifiers#3095

Merged
nighca merged 1 commit into
goplus:devfrom
aofei:completion
Apr 29, 2026
Merged

fix(code-editor): trigger completion for unicode identifiers#3095
nighca merged 1 commit into
goplus:devfrom
aofei:completion

Conversation

@aofei
Copy link
Copy Markdown
Member

@aofei aofei commented Apr 29, 2026

Allow completion to start when the typed character is a Go identifier letter or decimal digit outside ASCII. This lets resource names such as Chinese sprite names request suggestions after IME input.

Allow completion to start when the typed character is a Go identifier
letter or decimal digit outside ASCII. This lets resource names such as
Chinese sprite names request suggestions after IME input.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>
Copy link
Copy Markdown
Contributor

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

Updates the XGo code editor’s completion trigger logic to recognize non-ASCII Go identifier characters, improving completion behavior after IME input (e.g., Chinese sprite/resource names).

Changes:

  • Expand shouldTriggerCompletion from an ASCII-focused \w regex to a Unicode-aware regex using \p{L} / \p{Nd} with the u flag.

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

Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

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

The fix correctly addresses the stated goal — Unicode letters (e.g. Chinese sprite names) now trigger completion after IME input. A few observations:

Regex not hoisted to module scope
The regex literal inside shouldTriggerCompletion is re-evaluated on every call. Unicode property escapes (\p{L}) involve constructing internal Unicode character-category tables, which is more expensive than the ASCII \w shorthand. Hoisting the regex to module scope ensures it is compiled exactly once.

wordPattern not configured for Unicode
shouldTriggerCompletion now returns true for Chinese characters, but the subsequent call to textDocument.getWordAtPosition(position) (line 43) falls back to Monaco's default word pattern, which only recognises ASCII \w characters. As a result, when the cursor is after a Unicode identifier, wordStart will equal the current cursor position rather than the start of the identifier — completion is triggered, but the filter prefix and replacement range will be wrong. A custom wordPattern: /[\p{L}\p{Nd}_]+/u (or equivalent) should be added to xgoLanguageConfiguration to fully realise this fix.

\p{Nd} over-matches non-ASCII decimal digits
Go identifiers only permit ASCII decimal digits (0-9); the Go spec uses unicode_letter | unicode_digit but decimal_digit = "0" … "9". \p{Nd} also matches Arabic-Indic digits (٠١٢…), Devanagari digits (०१२…), etc. Using [0-9] instead of \p{Nd} would be a more faithful representation of the Go spec, though the current over-match is harmless (it triggers completion on characters that would be invalid in a Go identifier).

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the shouldTriggerCompletion function in the code editor's completion logic to use a Unicode-aware regular expression. This change ensures that completion is correctly triggered for non-ASCII letters and digits, improving support for international characters. I have no feedback to provide.

@nighca nighca merged commit fda2a02 into goplus:dev Apr 29, 2026
8 checks passed
@aofei aofei deleted the completion branch April 29, 2026 07:11
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.

3 participants