Skip to content

Conversation

@xterao
Copy link
Collaborator

@xterao xterao commented May 15, 2025

When performing code completion within a for directive block, the element names you define yourself are also suggested.
Elements within the same for directive are excluded.

@xterao xterao added this to the 0.7.0 Release milestone May 15, 2025
@xterao xterao requested a review from Copilot May 15, 2025 01:59
@xterao xterao self-assigned this May 15, 2025
@xterao xterao added bug Something isn't working fix Bug fixes labels May 15, 2025
Copy link
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

This PR aims to improve SQL code completion by ensuring that element names defined within a for directive block are not redundantly suggested during code completion.

  • Test case updated to cover additional suggestions including "member" and a special token "%for".
  • The for directive item retrieval now uses a primary expression rather than an identifier.
  • Filtering logic in the parameter completion provider is refined to exclude items from the current for directive context.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/test/kotlin/org/domaframework/doma/intellij/complate/sql/SqlCompleteTest.kt Updated expected suggestion list to include extra element names for a more complete test of for directive exclusion.
src/main/kotlin/org/domaframework/doma/intellij/extension/psi/SqlElForDirectiveExtension.kt Changed retrieval from SqlElIdExpr to SqlElPrimaryExpr to better capture the intended for directive item.
src/main/kotlin/org/domaframework/doma/intellij/contributor/sql/provider/SqlParameterCompletionProvider.kt Modified logic to filter out for directive items that belong to the current context, preventing self-suggestion.
Comments suppressed due to low confidence (3)

src/test/kotlin/org/domaframework/doma/intellij/complate/sql/SqlCompleteTest.kt:292

  • Ensure that additional test cases are added to verify that element names defined within a for directive are properly excluded from code completion suggestions.
listOf("employee", "member", "%for")

src/main/kotlin/org/domaframework/doma/intellij/contributor/sql/provider/SqlParameterCompletionProvider.kt:369

  • [nitpick] Consider adding a brief comment explaining the rationale behind filtering out for directive items that match the current context, to aid future maintainers.
val parentForDirectiveExpr = PsiTreeUtil.getParentOfType(position, SqlElForDirective::class.java) ?: PsiTreeUtil.getChildOfType(position.parent, SqlElForDirective::class.java)

src/main/kotlin/org/domaframework/doma/intellij/extension/psi/SqlElForDirectiveExtension.kt:34

  • Confirm that switching from SqlElIdExpr to SqlElPrimaryExpr correctly returns the intended for directive item without affecting downstream logic.
.getChildOfType(this, SqlElPrimaryExpr::class.java)

@xterao xterao merged commit 6db946f into main May 15, 2025
3 checks passed
@xterao xterao deleted the fix/code-completion-fix-for-directive-element-name branch May 15, 2025 02:26
@xterao xterao linked an issue May 15, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fix Bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code completion fix for directive element name

2 participants