From 8f4ee917b267054653f4dd5ec4aec3d1075fa7bd Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 23 Sep 2014 16:43:07 -0700 Subject: [PATCH 1/5] remove external module from showing up on completion list --- src/services/services.ts | 9 ++++++++- .../fourslash/completionListWithAmbientDeclaration.ts | 10 ++++++++++ tests/cases/fourslash/fourslash.ts | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/completionListWithAmbientDeclaration.ts diff --git a/src/services/services.ts b/src/services/services.ts index 04e63f6dbe1..fc9eb3e722d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1687,7 +1687,7 @@ module ts { if (displayName && displayName.length >= 2 && firstCharCode === displayName.charCodeAt(displayName.length - 1) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { // If the user entered name for the symbol was quoted, removing the quotes is not enough, as the name could be an - // invalid identifer name. We need to check if whatever was inside the quotes is actually a valid identifier name. + // invalid identifier name. We need to check if whatever was inside the quotes is actually a valid identifier name. displayName = displayName.substring(1, displayName.length - 1); } @@ -1708,6 +1708,13 @@ module ts { // Try to get a valid display name for this symbol, if we could not find one, then ignore it. // We would like to only show things that can be added after a dot, so for instance numeric properties can // not be accessed with a dot (a.1 <- invalid) + var firstCharCode = symbol.name.charCodeAt(0); + if ((symbol.flags & SymbolFlags.Namespace) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { + // If the symbol is external module, don't retry the completion list + // Also name of modules is invalid in completion list (i.e declare module "http" { var x; } | // <= request completion here, "http" should not be there) + return undefined; + } + var displayName = getValidCompletionEntryDisplayName(symbol.getName(), program.getCompilerOptions().target); if (!displayName) { return undefined; diff --git a/tests/cases/fourslash/completionListWithAmbientDeclaration.ts b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts new file mode 100644 index 00000000000..f67ebfdf434 --- /dev/null +++ b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts @@ -0,0 +1,10 @@ +/// + +//// declare module "http" { +//// var x; +//// } +//// /**/ + +goTo.marker(); +debugger +verifyNegatable.completionListContains("http"); \ No newline at end of file diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index f9476ee4d92..27caa0afe12 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -630,6 +630,7 @@ function verifyOperationIsCancelled(f) { var test = new FourSlashInterface.test_(); var goTo = new FourSlashInterface.goTo(); var verify = new FourSlashInterface.verify(); +var verifyNegatable = new FourSlashInterface.verifyNegatable(true); var edit = new FourSlashInterface.edit(); var debug = new FourSlashInterface.debug(); var format = new FourSlashInterface.format(); From c0eeafb0d8ee02e17a47398ced4ee5107543b9cb Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 23 Sep 2014 16:59:42 -0700 Subject: [PATCH 2/5] Update comments --- src/services/services.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index fc9eb3e722d..c91d2d79f6e 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1710,8 +1710,8 @@ module ts { // not be accessed with a dot (a.1 <- invalid) var firstCharCode = symbol.name.charCodeAt(0); if ((symbol.flags & SymbolFlags.Namespace) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { - // If the symbol is external module, don't retry the completion list - // Also name of modules is invalid in completion list (i.e declare module "http" { var x; } | // <= request completion here, "http" should not be there) + // If the symbol is external module, don't show it in the completion list + // (i.e declare module "http" { var x; } | // <= request completion here, "http" should not be there) return undefined; } From b37e8fce41e37515d7b5e31b6066e24ca64fd47e Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 23 Sep 2014 17:05:37 -0700 Subject: [PATCH 3/5] Remove debugger flag --- tests/cases/fourslash/completionListWithAmbientDeclaration.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cases/fourslash/completionListWithAmbientDeclaration.ts b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts index f67ebfdf434..b07c79f8390 100644 --- a/tests/cases/fourslash/completionListWithAmbientDeclaration.ts +++ b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts @@ -6,5 +6,4 @@ //// /**/ goTo.marker(); -debugger verifyNegatable.completionListContains("http"); \ No newline at end of file From 835d0ac551e34028b481d9614adc26c29b579e0d Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 23 Sep 2014 17:58:53 -0700 Subject: [PATCH 4/5] Address code review --- src/services/services.ts | 20 ++++++++++--------- .../completionListWithAmbientDeclaration.ts | 2 +- tests/cases/fourslash/fourslash.ts | 1 - 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index c91d2d79f6e..74e1eb6cf98 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1681,9 +1681,17 @@ module ts { } /// Completion - function getValidCompletionEntryDisplayName(displayName: string, target: ScriptTarget): string { + function getValidCompletionEntryDisplayName(symbol: Symbol, target: ScriptTarget): string { + var displayName = symbol.getName(); if (displayName && displayName.length > 0) { var firstCharCode = displayName.charCodeAt(0); + // First check of the displayName is not external module; if it is an external module, it is not valid entry + if ((symbol.flags & SymbolFlags.Namespace) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { + // If the symbol is external module, don't show it in the completion list + // (i.e declare module "http" { var x; } | // <= request completion here, "http" should not be there) + return undefined; + } + if (displayName && displayName.length >= 2 && firstCharCode === displayName.charCodeAt(displayName.length - 1) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { // If the user entered name for the symbol was quoted, removing the quotes is not enough, as the name could be an @@ -1696,6 +1704,7 @@ module ts { isValid = isIdentifierPart(displayName.charCodeAt(i), target); } + if (isValid) { return displayName; } @@ -1708,14 +1717,7 @@ module ts { // Try to get a valid display name for this symbol, if we could not find one, then ignore it. // We would like to only show things that can be added after a dot, so for instance numeric properties can // not be accessed with a dot (a.1 <- invalid) - var firstCharCode = symbol.name.charCodeAt(0); - if ((symbol.flags & SymbolFlags.Namespace) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { - // If the symbol is external module, don't show it in the completion list - // (i.e declare module "http" { var x; } | // <= request completion here, "http" should not be there) - return undefined; - } - - var displayName = getValidCompletionEntryDisplayName(symbol.getName(), program.getCompilerOptions().target); + var displayName = getValidCompletionEntryDisplayName(symbol, program.getCompilerOptions().target); if (!displayName) { return undefined; } diff --git a/tests/cases/fourslash/completionListWithAmbientDeclaration.ts b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts index b07c79f8390..fd00daab82d 100644 --- a/tests/cases/fourslash/completionListWithAmbientDeclaration.ts +++ b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts @@ -6,4 +6,4 @@ //// /**/ goTo.marker(); -verifyNegatable.completionListContains("http"); \ No newline at end of file +verify.not.completionListContains("http"); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 27caa0afe12..f9476ee4d92 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -630,7 +630,6 @@ function verifyOperationIsCancelled(f) { var test = new FourSlashInterface.test_(); var goTo = new FourSlashInterface.goTo(); var verify = new FourSlashInterface.verify(); -var verifyNegatable = new FourSlashInterface.verifyNegatable(true); var edit = new FourSlashInterface.edit(); var debug = new FourSlashInterface.debug(); var format = new FourSlashInterface.format(); From 684de531430336671b2e3bdc8ba40cb4cfba076e Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 24 Sep 2014 14:51:23 -0700 Subject: [PATCH 5/5] Address code review : add test case for inside ambient module declaration and single quote --- .../fourslash/completionListWithAmbientDeclaration.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/cases/fourslash/completionListWithAmbientDeclaration.ts b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts index fd00daab82d..afb5d96fbb7 100644 --- a/tests/cases/fourslash/completionListWithAmbientDeclaration.ts +++ b/tests/cases/fourslash/completionListWithAmbientDeclaration.ts @@ -2,8 +2,14 @@ //// declare module "http" { //// var x; +//// /*1*/ //// } -//// /**/ +//// declare module 'https' { +//// } +//// /*2*/ -goTo.marker(); +goTo.marker("1"); verify.not.completionListContains("http"); +goTo.marker("2"); +verify.not.completionListContains("http"); +verify.not.completionListContains("https");