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

Adding preparePasteEdits method to check if smart copy/paste should be applied #60053

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

navya9singh
Copy link
Member

@navya9singh navya9singh commented Sep 25, 2024

This pr adds a preparePasteEdits api to check if pasteEdits should be performed. #59881

One drawback here is, a case where the copied text is export const a = 1;, this would return true, because it checks if an identifier is exported in the current source file. This is needed, so that a case like

export const a = 1;
[|const b = a;|]

where const b = a is the copied text, can be true.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Sep 25, 2024
@typescript-bot
Copy link
Collaborator

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary.

@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@@ -2300,6 +2303,16 @@ export function createLanguageService(
};
}

function preparePasteEditsForFile(fileName: string, copiedTextRange: TextRange[]): boolean {
const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName);
bindSourceFile(sourceFile, getDefaultCompilerOptions());
Copy link
Member

Choose a reason for hiding this comment

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

You are not suppose to bind the file on syntax server. Also the options used here are wrong as they should be host.getCompilationSettings()

Copy link
Member Author

Choose a reason for hiding this comment

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

Without the binding, I don't get any locals or symbol.exports in the sourceFile. Is there a different way of doing this?

Copy link
Member

Choose a reason for hiding this comment

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

If thats the case you would need this command on "partial semantic" or "semantic server" instead

@navya9singh navya9singh marked this pull request as ready for review September 26, 2024 16:35
Copy link
Contributor

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Protocol changes look good. Thanks for looking into this! Will adopt it on the VS Code side once merged

@navya9singh navya9singh merged commit 8499803 into microsoft:main Sep 26, 2024
30 checks passed
mjbvz added a commit to mjbvz/vscode that referenced this pull request Sep 27, 2024
mjbvz added a commit to microsoft/vscode that referenced this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add preparePaste method to check if copied text should potentially have smart paste enabled or not
4 participants