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

Allow plugins to provide a list of external files. #15308

Merged
merged 1 commit into from Jun 5, 2017

Conversation

chuckjaz
Copy link
Contributor

The list of the plugin's external files and request made to
a file in the list will be routed to an instance of the plugin.

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

My super annoying lint nits

// by the LSHost for files in the program when the program is retrieved above but
// the program doesn't contain external files so this must be done explicitly.
inserted => {
const scriptInfo = this.projectService.getOrCreateScriptInfo(inserted, false);
Copy link
Member

Choose a reason for hiding this comment

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

15:47:16 src/server/project.ts
15:47:16 ERROR: 579:92  boolean-trivia  Tag boolean argument with parameter name

const bItem = b[bIndex];
const compareResult = compare(aItem, bItem);
if (compareResult < 0) {
inserted(aItem)
Copy link
Member

Choose a reason for hiding this comment

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

15:47:16 src/server/utilities.ts
15:47:16 ERROR: 204:32  semicolon       Missing semicolon

export function enumerateInsertsAndDeletes<T>(a: SortedReadonlyArray<T>, b: SortedReadonlyArray<T>, inserted: (item: T) => void, deleted: (item: T) => void, compare?: (a: T, b: T) => number) {
if (!compare) compare = (a, b) => a > b ? 1 : a < b ? -1 : 0;
let aIndex = 0, aLen = a.length;
let bIndex = 0, bLen = b.length;
Copy link
Member

Choose a reason for hiding this comment

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

15:47:16 src/server/utilities.ts
15:47:16 ERROR: 197:25  prefer-const    Identifier 'aLen' is never reassigned; use 'const' instead of 'let'.
15:47:16 ERROR: 198:25  prefer-const    Identifier 'bLen' is never reassigned; use 'const' instead of 'let'.

@@ -192,6 +192,33 @@ namespace ts.server {
return <any>arr;
}

export function enumerateInsertsAndDeletes<T>(a: SortedReadonlyArray<T>, b: SortedReadonlyArray<T>, inserted: (item: T) => void, deleted: (item: T) => void, compare?: (a: T, b: T) => number) {
if (!compare) compare = (a, b) => a > b ? 1 : a < b ? -1 : 0;
Copy link
Member

Choose a reason for hiding this comment

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

Curlies, also consider

compare = compare || ts.compareValues

if its semantics work well enough on undefined values.

},
removed => {
const scriptInfoToDetach = this.projectService.getScriptInfo(removed);
if (scriptInfoToDetach)
Copy link
Member

Choose a reason for hiding this comment

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

curlies

getExternalFiles(): string[] {
return [];
getExternalFiles(): SortedReadonlyArray<string> {
return [] as any as SortedReadonlyArray<string>;
Copy link
Member

Choose a reason for hiding this comment

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

Could you not just use emptyArray?

if (compareResult < 0) {
inserted(aItem)
aIndex++;
} else if (compareResult > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

elses on next line

@RyanCavanaugh
Copy link
Member

Looks like some lint failures (run jake lint to see a quick list)

@chuckjaz
Copy link
Contributor Author

@RyanCavanaugh For some reason I cannot get jake lint to run on Mac. Is this a known issue?

@RyanCavanaugh
Copy link
Member

@chuckjaz I'm not aware of any problems. What's the console output?

@chuckjaz
Copy link
Contributor Author

@RyanCavanaugh I found out what it is. I had a very old node_module and npm didn't update the tslint I had for some reason. I removed it and reinstalled and it worked.

The list of the plugin's external files and request made to
a file in the list will be routed to an instance of the plugin.
@fxck
Copy link

fxck commented May 17, 2017

Any update on this?

@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

@RyanCavanaugh can you please review this change

@RyanCavanaugh
Copy link
Member

LGTM

@RyanCavanaugh
Copy link
Member

@mhegazy please merge once you've given a quick sanity check as well

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.

None yet

6 participants