Skip to content

fixed in_rule_line, in_reduce_action bool combination into LocationType enum - #87

Merged
ehwan merged 3 commits into
mainfrom
lsp_location_type
Jun 27, 2026
Merged

fixed in_rule_line, in_reduce_action bool combination into LocationType enum#87
ehwan merged 3 commits into
mainfrom
lsp_location_type

Conversation

@ehwan

@ehwan ehwan commented Jun 27, 2026

Copy link
Copy Markdown
Owner

LocationType:

  • Outside
  • NonTerminalDefintiion
  • ProductionLine
  • ReduceAction

@ehwan ehwan self-assigned this Jun 27, 2026
@ehwan
ehwan merged commit 2175afa into main Jun 27, 2026
1 check passed
@ehwan
ehwan deleted the lsp_location_type branch June 27, 2026 11:51

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

Copy link
Copy Markdown
Contributor

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 refactors the LSP completion and hover logic by replacing boolean flags with a more precise LocationType enum (Outside, NonTerminalDefinition, ProductionLine, ReduceAction). This change enables context-aware completions and hovers, preventing irrelevant suggestions such as directives inside reduce actions. Feedback on this PR highlights a regression where hovering over global substitution variables (e.g., $num or $Expr) inside a non-terminal definition header returns None because hover documentation lookups are now restricted to LocationType::ReduceAction. A fix is suggested to allow dollar-name substitution lookups when the location type is LocationType::NonTerminalDefinition.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +77 to +85
documentation = match location_type {
LocationType::ReduceAction => reduce_action_variable_documentation(args, &word)
.or_else(|| {
reduce_action_reference_documentation_for_word(args, content, offset, &word)
}),
LocationType::Outside
| LocationType::NonTerminalDefinition
| LocationType::ProductionLine => None,
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Hovering over global substitution variables (such as $num or $Expr) in a non-terminal definition header (e.g., Expr($num)) will return None because reduce_action_reference_documentation_for_word is only called when location_type is LocationType::ReduceAction.

To fix this regression, we should allow calling dollar_name_substitution_documentation when location_type is LocationType::NonTerminalDefinition and the hovered word starts with $ (excluding numeric positional variables).

            documentation = match location_type {
                LocationType::ReduceAction => reduce_action_variable_documentation(args, &word)
                    .or_else(|| {
                        reduce_action_reference_documentation_for_word(args, content, offset, &word)
                    }),
                LocationType::NonTerminalDefinition => {
                    if word.starts_with('$') && word.strip_prefix('$').and_then(|s| s.parse::<usize>().ok()).is_none() {
                        dollar_name_substitution_documentation(args, content, &word)
                    } else {
                        None
                    }
                }
                LocationType::Outside | LocationType::ProductionLine => None,
            };

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.

1 participant