-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: not complete str functions when inside literal str #923
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: he1pa <[email protected]>
c050175
to
5190392
Compare
Pull Request Test Coverage Report for Build 7013945867
💛 - Coveralls |
@CodiumAI-Agent /review |
PR Analysis
PR Feedback
How to useInstructions
|
@CodiumAI-Agent /improve |
PR Code Suggestions💡 Suggestion: Consider refactoring the completion function to reduce its complexity and improve readability. The function is currently doing too much, which makes it hard to understand. You could extract the logic for handling different types of expressions into separate functions. File: kclvm/tools/src/LSP/src/completion.rs (169-198) Example code:Existing code: fn completion_dot(
...
Stmt::Import(stmt) => return completion_import(&stmt, &pre_pos, prog_scope, program),
_ => {
// Todo: string lit has not been processed using the new semantic model and need to handle here.
let (expr, _) = inner_most_expr_in_stmt(&stmt.node, &pre_pos, None);
if let Some(node) = expr {
if let Expr::StringLit(_) = node.node {
if pre_pos == node.get_end_pos() {
return Some(
into_completion_items(
&STRING_MEMBER_FUNCTIONS
.iter()
.map(|(name, ty)| KCLCompletionItem {
label: func_ty_complete_label(
name,
&ty.into_function_ty(),
),
detail: Some(ty.ty_str()),
documentation: ty.ty_doc(),
kind: Some(KCLCompletionItemKind::Function),
})
.collect(),
)
.into(),
);
} else {
return Some(into_completion_items(&items).into());
}
}
}
}
) Improved code: fn completion_dot(
...
Stmt::Import(stmt) => return completion_import(&stmt, &pre_pos, prog_scope, program),
_ => {
return handle_other_statements(&stmt, &pre_pos);
}
)
fn handle_other_statements(stmt: &Stmt, pre_pos: &KCLPos) -> Option<CompletionResponse> {
// Todo: string lit has not been processed using the new semantic model and need to handle here.
let (expr, _) = inner_most_expr_in_stmt(&stmt.node, pre_pos, None);
if let Some(node) = expr {
if let Expr::StringLit(_) = node.node {
return handle_string_literal(node, pre_pos);
}
}
None
}
fn handle_string_literal(node: &Expr, pre_pos: &KCLPos) -> Option<CompletionResponse> {
if pre_pos == node.get_end_pos() {
return Some(
into_completion_items(
&STRING_MEMBER_FUNCTIONS
.iter()
.map(|(name, ty)| KCLCompletionItem {
label: func_ty_complete_label(
name,
&ty.into_function_ty(),
),
detail: Some(ty.ty_str()),
documentation: ty.ty_doc(),
kind: Some(KCLCompletionItemKind::Function),
})
.collect(),
)
.into(),
);
} else {
return Some(into_completion_items(&items).into());
}
} |
Signed-off-by: he1pa <[email protected]>
60acc52
to
228f7d6
Compare
Pull Request Test Coverage Report for Build 7014853009
💛 - Coveralls |
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
lsp/completion.rs
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
not complete str functions when inside literal str
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links: