Skip to content

Conversation

@xterao
Copy link
Collaborator

@xterao xterao commented Apr 8, 2025

We have fixed an issue where, when performing code completion for the % directive, the candidate string was appended to the end of the input text. Now, the text up to % will be replaced by the candidate string.

@xterao xterao added the bug Something isn't working label Apr 8, 2025
@github-actions github-actions bot added the fix Bug fixes label Apr 8, 2025
@xterao xterao linked an issue Apr 8, 2025 that may be closed by this pull request
@xterao xterao self-assigned this Apr 8, 2025
@xterao xterao requested a review from Copilot April 8, 2025 04:25
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.

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

Comments suppressed due to low confidence (1)

src/main/kotlin/org/domaframework/doma/intellij/common/sql/directive/PercentDirectiveHandler.kt:36

  • [nitpick] The variable name 'beforeTextChars' may be misleading since it holds the count of previous leaf nodes rather than their character content. Consider renaming it to 'prevLeafCount' for improved clarity.
val beforeTextChars =

@xterao xterao force-pushed the fix/directive-complation branch from 8a991a0 to 4c23b67 Compare April 8, 2025 04:27
@xterao xterao requested a review from Copilot April 8, 2025 04:27
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.

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

Comments suppressed due to low confidence (1)

src/main/kotlin/org/domaframework/doma/intellij/common/sql/directive/PercentDirectiveHandler.kt:36

  • Counting PSI leaf nodes to determine the number of characters assumes that each leaf node represents one character, which may not be true. Consider summing the lengths of each leaf's text to accurately calculate the start offset for replacement.
val beforeTextChars = this.element.prevLeafs.takeWhile { prev -> prev.text != "%" }.toList().size

@xterao
Copy link
Collaborator Author

xterao commented Apr 8, 2025

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

Comments suppressed due to low confidence (1)
Counting PSI leaf nodes to determine the number of characters assumes that each leaf node represents one character, which
may not be true. Consider summing the lengths of each leaf's text to accurately calculate the start offset for replacement.

In this directive's code completion, it is assumed that the preceding string is retrieved as individual characters.

@xterao xterao merged commit c9601a4 into main Apr 8, 2025
4 checks passed
@xterao xterao deleted the fix/directive-complation branch April 8, 2025 04:38
@xterao xterao linked an issue Apr 8, 2025 that may be closed by this pull request
@xterao xterao linked an issue Apr 9, 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.

Fix: Code completion for conditional directives

2 participants