Skip to content

Move function typed properties from the SourceFile to a dedicated functions #1700

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 8 commits into from
Feb 3, 2015

Conversation

vladima
Copy link
Contributor

@vladima vladima commented Jan 17, 2015

Function typed properties on SourceFile are made with closures, for all source files it will keep the parser instance alive. This PR makes SourceFile pure data object -> no closures are required

// from this SourceFile that are being held onto may change as a result (including
// becoming detached from any SourceFile). It is recommended that this SourceFile not
// be used once 'update' is called on it.
export function update(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange): SourceFile {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe updateSourceFile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will change

@ahejlsberg
Copy link
Member

Looks good, but hard to see exactly what changed in the large blocks that moved.

@vladima
Copy link
Contributor Author

vladima commented Jan 17, 2015

Large blocks are mostly auxiliary function that were used by update. Previously these functions were nested in createSourceFile and now they were pushed in the outer scope

@@ -391,7 +391,7 @@ module FourSlash {
this.currentCaretPosition = pos;

var lineStarts = ts.computeLineStarts(this.getCurrentFileContent());
var lineCharPos = ts.getLineAndCharacterOfPosition(lineStarts, pos);
var lineCharPos = ts.computeLineAndCharacterOfPosition(lineStarts, pos);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between this and getLineAndCharacterOfPosition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

computeLineAndCharacterOfPosition accepts lineMap as number[] and use it to compute line and character. getLineAndCharacterOfPosition accepts SourceFile, picks lineMap from the file (and initializes it if it was not yet created) and then invokes computeLineAndCharacterOfPosition

@sheetalkamat
Copy link
Member

👍

vladima added a commit that referenced this pull request Feb 3, 2015
Move function typed properties from the SourceFile to a dedicated functions
@vladima vladima merged commit 626277c into master Feb 3, 2015
@vladima vladima deleted the noMethodsOnSourceFile branch February 3, 2015 21:22
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
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.

7 participants