Skip to content

textChanges: Clean up *Options interfaces #22813

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.

Sign up for GitHub

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
1 commit merged into from
Mar 26, 2018
Merged

textChanges: Clean up *Options interfaces #22813

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions src/services/textChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace ts.textChanges {
* By default when removing nodes we adjust start and end positions to respect specification of the trivia above.
* If pos\end should be interpreted literally 'useNonAdjustedStartPosition' or 'useNonAdjustedEndPosition' should be set to true
*/
export type ConfigurableStartEnd = ConfigurableStart & ConfigurableEnd;
export interface ConfigurableStartEnd extends ConfigurableStart, ConfigurableEnd {}

export const useNonAdjustedPositions: ConfigurableStartEnd = {
useNonAdjustedStartPosition: true,
Expand Down Expand Up @@ -113,13 +113,11 @@ namespace ts.textChanges {
readonly range: TextRange;
}

export interface ChangeNodeOptions extends ConfigurableStartEnd, InsertNodeOptions {
readonly useIndentationFromFile?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

I gather this property was unused?

Copy link
Author

Choose a reason for hiding this comment

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

Right.

}
export interface ChangeNodeOptions extends ConfigurableStartEnd, InsertNodeOptions {}
interface ReplaceWithSingleNode extends BaseChange {
readonly kind: ChangeKind.ReplaceWithSingleNode;
readonly node: Node;
readonly options?: ChangeNodeOptions;
readonly options?: InsertNodeOptions;
}

interface RemoveNode extends BaseChange {
Expand All @@ -131,7 +129,7 @@ namespace ts.textChanges {
interface ReplaceWithMultipleNodes extends BaseChange {
readonly kind: ChangeKind.ReplaceWithMultipleNodes;
readonly nodes: ReadonlyArray<Node>;
readonly options?: ChangeNodeOptions;
readonly options?: InsertNodeOptions;
}

function getAdjustedStartPosition(sourceFile: SourceFile, node: Node, options: ConfigurableStart, position: Position) {
Expand Down Expand Up @@ -283,7 +281,7 @@ namespace ts.textChanges {
}

// TODO (https://github.com/Microsoft/TypeScript/issues/21246): default should probably be useNonAdjustedPositions
public replaceRange(sourceFile: SourceFile, range: TextRange, newNode: Node, options: ChangeNodeOptions = {}) {
public replaceRange(sourceFile: SourceFile, range: TextRange, newNode: Node, options: InsertNodeOptions = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

As before, I personally find it strange to change parameter types of some, but not all, members of this family of functions.

Copy link
Author

Choose a reason for hiding this comment

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

interface ChangeNodeOptions extends ConfigurableStartEnd, InsertNodeOptions {}. ConfigurableStartEnd determines how the start and end positions of the node parameter are adjusted. This function doesn't take a node as a parameter, so those options would be unused.

this.changes.push({ kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range, options, node: newNode });
return this;
}
Expand All @@ -302,7 +300,7 @@ namespace ts.textChanges {
return this.replaceRange(sourceFile, { pos, end }, newNode, options);
}

public replaceRangeWithNodes(sourceFile: SourceFile, range: TextRange, newNodes: ReadonlyArray<Node>, options: ChangeNodeOptions = useNonAdjustedPositions) {
public replaceRangeWithNodes(sourceFile: SourceFile, range: TextRange, newNodes: ReadonlyArray<Node>, options: InsertNodeOptions = {}) {
this.changes.push({ kind: ChangeKind.ReplaceWithMultipleNodes, sourceFile, range, options, nodes: newNodes });
return this;
}
Expand Down Expand Up @@ -668,9 +666,7 @@ namespace ts.textChanges {
const initialIndentation =
options.indentation !== undefined
? options.indentation
: (options.useIndentationFromFile !== false)
? formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, options.prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos)
: 0;
: formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, options.prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
const delta =
options.delta !== undefined
? options.delta
Expand Down