Closed shruti2522 closed 3 weeks ago
Hello @shruti2522
Please note that the position of <cursor>
I mentioned in the issue is a completion feature of dot ".", not a completion of the attribute value list.
cc @He1pa Could you help review it?
Hello @shruti2522
Please note that the position of
<cursor>
I mentioned in the issue is a completion feature of dot ".", not a completion of the attribute value list.cc @He1pa Could you help review it?
Okay got it @Peefy , I will push the code for dot completion. Should I undo the attribute completion logic here?
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
---|---|---|---|---|---|
kclvm/sema/src/core/symbol.rs | 0 | 6 | 0.0% | ||
<!-- | Total: | 0 | 6 | 0.0% | --> |
Files with Coverage Reduction | New Missed Lines | % | ||
---|---|---|---|---|
kclvm/sema/src/core/symbol.rs | 3 | 49.52% | ||
<!-- | Total: | 3 | --> |
Totals | |
---|---|
Change from base Build 9394743478: | 0.0% |
Covered Lines: | 55030 |
Relevant Lines: | 77425 |
Okay got it @Peefy , I will push the code for dot completion. Should I undo the attribute completion logic here?
Sure. Undo it
Hello @shruti2522
Please note that the position of
<cursor>
I mentioned in the issue is a completion feature of dot ".", not a completion of the attribute value list.
Done @Peefy
Hello. You can just add symbol complete items here for the string lit type 😄: https://github.com/kcl-lang/kcl/blob/main/kclvm/sema/src/core/symbol.rs#L385
Hello. You can just add symbol complete items here for the string lit type 😄: https://github.com/kcl-lang/kcl/blob/main/kclvm/sema/src/core/symbol.rs#L385
Hello @Peefy, I tried using get_type_all_attribute
earlier, but it needs the name
argument, which requires the string symbol def to be defined first. So, I used get_type_symbol
instead because it doesn't require the name arg.
Hello. You can just add symbol complete items here for the string lit type 😄: https://github.com/kcl-lang/kcl/blob/main/kclvm/sema/src/core/symbol.rs#L385
Hello @Peefy, I tried using
get_type_all_attribute
earlier, but it needs thename
argument, which requires the string symbol def to be defined first. So, I usedget_type_symbol
instead because it doesn't require the name arg.
cc @He1pa Could you help give more comments?
Do I need to make any changes to the current approach @Peefy @He1pa ?
Do I need to make any changes to the current approach @Peefy @He1pa ?
Yes.
Hello, I attempted to integrate the get_type_all_attribute
function. In my first approach, I extracted the symbol definition for the union type and passed the definition name to the argument. In the second approach, I passed an empty string to the name argument. However, in both cases, failure occurs after one successful run. Could you please review the code? @Peefy
let sema_info = def.get_sema_info();
if let Some(ty) = &sema_info.ty {
let ty_symbl_refs = gs
.get_symbols()
.get_type_symbol(ty, gs.get_packages().get_module_info(&pos.filename));
if let Some(ty_symbl_ref) = ty_symbl_refs {
if let Some(ty_symbl_def) = gs.get_symbols().get_symbol(ty_symbl_ref) {
if let TypeKind::Union(union_types) = &ty.kind {
for union_ty in union_types {
if let TypeKind::StrLit(_) | TypeKind::Str = &union_ty.kind
{
let str_attrs =
gs.get_symbols().get_type_all_attribute(
&union_ty,
&ty_symbl_def.get_name(),
gs.get_packages()
.get_module_info(&pos.filename),
);
let sema_info = def.get_sema_info();
if let Some(ty) = &sema_info.ty {
if let TypeKind::Union(union_types) = &ty.kind {
for union_ty in union_types {
if let TypeKind::StrLit(_) | TypeKind::Str = &union_ty.kind {
let module_info =
gs.get_packages().get_module_info(&pos.filename);
let str_attrs = gs.get_symbols().get_type_all_attribute(
union_ty,
"",
module_info
);
current behaviour:
cc @He1pa Could you help review it?
done @Peefy
Do I need to make any further changes @Peefy @He1pa ?
Hello @shruti2522
I think just modifying the symbol.rs file is enough, there is no need to modify the completion.rs
file. The correct modification method only requires two steps
get_type_symbol
function TypeKind::Union(types) => {
if types.iter().all(|ut| {
matches!(
&ut.kind,
TypeKind::StrLit(_) | TypeKind::Str
)
}) {
self.get_symbol_by_fully_qualified_name(BUILTIN_STR_PACKAGE)
} else {
None
}
}
get_type_all_attribute
function TypeKind::StrLit(_) | TypeKind::Str => {
let mut result = vec![];
if let Some(symbol_ref) = self.get_type_symbol(ty, module_info) {
if let Some(symbol) = self.get_symbol(symbol_ref) {
result = symbol.get_all_attributes(self, module_info);
}
}
result
}
Besides, you need to add more unit tests for this feature.
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):
kclvm/tools/src/LSP/src/completion.rs
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
Screencast from 2024-06-06 21-56-13.webm
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: