Skip to content
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

lsp: Rename components, structs and enum from places where they are used #7305

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hunger
Copy link
Member

@hunger hunger commented Jan 8, 2025

This has pretty decent test coverage, but it is more involved than the first and it is admittedly a bit of a mess in the PR...

I was test driven and forgot to stop along the way ;-)

@hunger
Copy link
Member Author

hunger commented Jan 8, 2025

It mostly works when testing from VSCode, but it still has a couple of rough edges where the tests pass, but VSCode does not want to trigger. I'll poke at it a bit more tomorrow.

.child_token(SyntaxKind::StringLiteral)
.map(|t| t.text().trim_matches('"').to_string())?;

if import == "std-widgets.slint" || import == "std-widgets.s60" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if import == "std-widgets.slint" || import == "std-widgets.s60" {
if import == "std-widgets.slint" {

It was called sixtyfps_widgets.60 but we no longer support that

Foo /* <- TEST_ME_9 */ { }
}

function Foo(Foo: int) { Foo + 1; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should also test that none of the Foo here matches with a component.

}

impl DeclarationNodeQuery {
fn new(node: SyntaxNode) -> Option<Self> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it is an incomplete duplication of language::token_info::token_info()
That function also do the lookup and tell you the exact type this is referring to.
So I think it would be a better candidate.

@hunger
Copy link
Member Author

hunger commented Jan 9, 2025

I started to pull out a bit of code from here into easier to review portions in separate PRs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants