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

Merged
merged 3 commits into from
Jan 17, 2025

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.

tools/lsp/common/rename_component.rs Outdated Show resolved Hide resolved
tools/lsp/common/rename_component.rs Show resolved Hide resolved
tools/lsp/common/rename_component.rs Outdated Show resolved Hide resolved
@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...

@hunger hunger force-pushed the tobias/push-yypvkoyqrqko branch from 9b6416d to e517dc8 Compare January 16, 2025 21:03
Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

(partial review)

Nice to see this coming along.

Please make sure there are also tests that control that things that have the same name but are different (eg, a struct and a property) are not renamed.
Apologies is this is already there and I missed it.

tools/lsp/common/token_info.rs Outdated Show resolved Hide resolved
tools/lsp/common/rename_component.rs Outdated Show resolved Hide resolved
@hunger hunger force-pushed the tobias/push-yypvkoyqrqko branch from e517dc8 to d7af605 Compare January 17, 2025 08:20
Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

Nice work!
Good to see this finally done.

@hunger
Copy link
Member Author

hunger commented Jan 17, 2025

I'll do one more round through CI: I seem to have missed a test for renaming a component with an enum of the same name in scope.

... and their usages and imports/exports

Use the token_info to do this.
@hunger hunger force-pushed the tobias/push-yypvkoyqrqko branch from d7af605 to aba42fa Compare January 17, 2025 08:40
@hunger hunger merged commit f4b0922 into slint-ui:master Jan 17, 2025
37 checks passed
@hunger hunger deleted the tobias/push-yypvkoyqrqko branch January 17, 2025 09:27
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