Skip to content

ChangeTracker: Combine deleteDeclaration, deleteNode, and deleteNodeInList#25276

Merged
3 commits merged into
masterfrom
changes_delete
Jun 30, 2018
Merged

ChangeTracker: Combine deleteDeclaration, deleteNode, and deleteNodeInList#25276
3 commits merged into
masterfrom
changes_delete

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jun 27, 2018

Sequel to #25011 -- that added a new deletion method. As of this PR, you can just call delete and ChangeTracker should be smart enough to know what kind of deletion to do.

@ghost ghost requested review from amcasey and mhegazy June 27, 2018 18:03
@ghost ghost force-pushed the changes_delete branch from 914833c to 8834e87 Compare June 27, 2018 18:30
Comment thread src/services/textChanges.ts Outdated
this.deleteRange(sourceFile, { pos: startPosition, end: endPosition });
return this;
delete(sourceFile: SourceFile, node: Node): void {
this.deletedDeclarations.push({ sourceFile, node, });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i would call this deletedNodes, since nothing ensures they are a declaration

@ghost ghost merged commit ded4465 into master Jun 30, 2018
@ghost ghost deleted the changes_delete branch June 30, 2018 02:53
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant