Skip to content

fix: number of prepend lines sent to LSP #14

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

Merged
merged 2 commits into from
Jun 16, 2025
Merged

Conversation

heyzec
Copy link
Collaborator

@heyzec heyzec commented Jun 16, 2025

The extension tells the LSP server how many lines of code at prepended, which the LSP server uses to determine where to insert imports. The current implementation is bugged and does not consider the marker lines, causing the LSP to add auto imports inside the prepend section instead of below.

@heyzec
Copy link
Collaborator Author

heyzec commented Jun 16, 2025

We can consider removing the nPrependLines argument being passed to the LSP. Instead, the LSP uses the end marker to determine where to add its imports.

@heyzec heyzec force-pushed the fix-n-prepend-lines branch from 01e66e1 to 93808ad Compare June 16, 2025 12:08
@heyzec heyzec requested review from mug1wara26 June 16, 2025 12:09
@mug1wara26
Copy link
Collaborator

lgtm, my only concern with the lsp auto detecting the end of prepend is if there are no prepended lines, the current change does not place any markers right? If there was no prepend marker, the user could insert their own and lead to unexpected behaviour. This isn't too big of an issue cause its very unlikely someone would put // END PREPEND in their own code, but it is still not intended behaviour.

@mug1wara26 mug1wara26 enabled auto-merge (squash) June 16, 2025 12:39
@mug1wara26 mug1wara26 merged commit c248b20 into main Jun 16, 2025
2 checks passed
@mug1wara26 mug1wara26 deleted the fix-n-prepend-lines branch June 16, 2025 12:40
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