-
Notifications
You must be signed in to change notification settings - Fork 121
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
feat: add hints for schema config expr #1589
Conversation
Hello @shruti2522 What I mean in #1575 is to add hints to schema arguments, just like function parameters, rather than schema config expr. |
Okay. I was initially working on adding hints for this, so I misunterstood. Will add hints for schema arguments. From what I understand it is supposed to be like this? @Peefy @He1pa
|
@shruti2522 Examples of schema arguments schema Person[age: int, name: str]:
a = age
n = name
p = Person([age: ]1, [name: ]"Alice") # Add hints here |
Of course, adding hints to the schema config expr is also possible. You can open a new issue and associate it with it. |
These are two features. Do not associate this PR with #1575, but #1244. |
Signed-off-by: shruti2522 <shruti.apc01@gmail.com>
Signed-off-by: shruti2522 <shruti.apc01@gmail.com>
101043b
to
422aa4c
Compare
currently, hints are also added to schema config where schema name is alraedy defined, for example, Since config_expr doesn't have a schema name field associated with it, how can we add a check for the name declaration here? Should I do a schema_expr check here or maybe check for identifier expr. |
cc @He1pa |
Signed-off-by: shruti2522 <shruti.apc01@gmail.com> schema name hint Signed-off-by: shruti2522 <shruti.apc01@gmail.com> set sema type Signed-off-by: shruti2522 <shruti.apc01@gmail.com> add expr name Signed-off-by: shruti2522 <shruti.apc01@gmail.com> revert change in fib.k Signed-off-by: shruti2522 <shruti.apc01@gmail.com> conditional hint Signed-off-by: shruti2522 <shruti.apc01@gmail.com> add tests Signed-off-by: shruti2522 <shruti.apc01@gmail.com> add config_entry Signed-off-by: shruti2522 <shruti.apc01@gmail.com> add hints for type assign Signed-off-by: shruti2522 <shruti.apc01@gmail.com> add hint Signed-off-by: shruti2522 <shruti.apc01@gmail.com>
8bc62ac
to
9859903
Compare
Signed-off-by: shruti2522 <shruti.apc01@gmail.com>
Pull Request Test Coverage Report for Build 10597220055Details
💛 - Coveralls |
Signed-off-by: shruti2522 <shruti.apc01@gmail.com>
column: start.column.map(|c| if c >= 1 { c - 1 } else { 0 }), | ||
}; | ||
|
||
let with_hint = if let Some(stmt) = self.ctx.program.pos_to_stmt(&pre_pos) { |
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.
The complexity of this function is a bit high, and other methods can be used to determine it.
Besides, why add hint only for the assign_stmt
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.
I guess hints have to applied only for anonymous schema configs, like
schema Name:
name: str
n: Name = {
name = "kcl"
}
that's why I used assign_stmt to determine if type annotations exist for the same, if yes then hints are applied, otherwise not.
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.
schema Name:
name: str
config: Config
schema Config:
count: int
n: Name = {
name = "foo"
config: {
count: 1
}
}
The config
in the Name
schema also needs hints e.g.
schema Name:
name: str
config: Config
schema Config:
count: int
n: Name = [Name ] { # Name schema hints
name = "foo"
config: [Config ] { # Config schema hints
count: 1
}
}
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.
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.
How about this code, an assign stmt with the type annotation and the right value is a schema expr.
schema Name:
name: str
config: Config
schema Config:
count: int
n Name = Name { # Name schema hints
name = "foo"
config: [Config ] { # Config schema hints
count: 1
}
}
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.
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.
What I mean is that there are many such grammars, not just checking schema_exp or assign_stmt, but there needs to be a more effective way.
Signed-off-by: shruti2522 <shruti.apc01@gmail.com>
Signed-off-by: shruti2522 <shruti.apc01@gmail.com>
Hello @shruti2522 Have you been working this issue? |
Yes @Peefy |
Hello @shruti2522 Have you been working this issue and PR? I will close this PR until next week. |
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
fix [LFX] [Track] LSP inlayhint and hover content content optimization #1244
2. What is the scope of this PR (e.g. component or file name):
kclvm/sema/src/advanced_resolver/node.rs
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
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: