Skip to content

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

Merged
RyanCavanaugh merged 1 commit into
microsoft:masterfrom
chuckjaz:external-files
Jun 5, 2017
Merged

Allow plugins to provide a list of external files.#15308
RyanCavanaugh merged 1 commit into
microsoft:masterfrom
chuckjaz:external-files

Conversation

@chuckjaz
Copy link
Copy Markdown
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
Copy Markdown
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

Comment thread src/server/project.ts Outdated
// 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
Copy Markdown
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

Comment thread src/server/utilities.ts Outdated
const bItem = b[bIndex];
const compareResult = compare(aItem, bItem);
if (compareResult < 0) {
inserted(aItem)
Copy link
Copy Markdown
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

Comment thread src/server/utilities.ts Outdated
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
Copy Markdown
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'.

Comment thread src/server/utilities.ts Outdated
}

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
Copy Markdown
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.

Comment thread src/server/project.ts Outdated
},
removed => {
const scriptInfoToDetach = this.projectService.getScriptInfo(removed);
if (scriptInfoToDetach)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

curlies

Comment thread src/server/project.ts Outdated
getExternalFiles(): string[] {
return [];
getExternalFiles(): SortedReadonlyArray<string> {
return [] as any as SortedReadonlyArray<string>;
Copy link
Copy Markdown
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?

Comment thread src/server/utilities.ts Outdated
if (compareResult < 0) {
inserted(aItem)
aIndex++;
} else if (compareResult > 0) {
Copy link
Copy Markdown
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
Copy Markdown
Member

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

@chuckjaz
Copy link
Copy Markdown
Contributor Author

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

@RyanCavanaugh
Copy link
Copy Markdown
Member

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

@chuckjaz
Copy link
Copy Markdown
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
Copy Markdown

fxck commented May 17, 2017

Any update on this?

@mhegazy mhegazy requested a review from RyanCavanaugh May 24, 2017 23:20
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 24, 2017

@RyanCavanaugh can you please review this change

@RyanCavanaugh
Copy link
Copy Markdown
Member

LGTM

@RyanCavanaugh
Copy link
Copy Markdown
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.

6 participants