Completion list show correct entry for function expression and class expression by yuit · Pull Request #3643 · microsoft/TypeScript

yuit · 2015-06-26T01:37:19Z

Fix #3231 and for class expression case

…ssion

DanielRosenwasser · 2015-06-26T03:05:49Z

Look into getDeclaredName instead, including the above for the default export

getDeclaredName doesn't handle function expression correctly. Talk with @mhegazy, the function needed some rewrite and I don't want to have it done in this PR. I will put a TODO here to have it change once getDeclaredName is done

@DanielRosenwasser should have a fix for this in a pending review. you should synchronize.

Ah right, it's in my PR. Speaking of which, can you guys review it? 😃

#3367

You should be clear to use getDeclaredName

🌹 yeah
🌵

mhegazy · 2015-06-26T17:43:06Z

note. you need to handle this on the managed side as well.

What you mean be handling in the managed side?

Probably best to discuss this in person with someone but I believe the idea is that VS needs to handle styles differently depending on what comes in to the managed side.

what do you see in navbar when you do this?

🌵 file a bug

DanielRosenwasser · 2015-06-29T16:37:07Z

@yuit if you pull in from master, there's a fourslash test called renameLocationsForClassExpression01.ts. You'll need to amend it to test correctly in light of the fact that you fixed class expression name resolution.

…with certain kind, document, and text is the completion list

DanielRosenwasser · 2015-07-01T22:29:02Z

Why not change the logic of copySymbol?

Also, the comment for copySymbol says // Returns 'true' if we should stop processing symbols., but it returns void. Can you fix that?

Remove the old comment

Conflicts: src/compiler/checker.ts

JsonFreeman · 2015-07-06T22:54:10Z

Nice, that is cute :)

…class expression

Conflicts: tests/cases/fourslash/renameLocationsForClassExpression01.ts

…t now correctly represent declared-name.

mhegazy · 2015-07-09T18:32:06Z

class expression listed twice

Uh oh. It'd be good to have a feature that catches this.

@JsonFreeman it's being tracked in #2854

mhegazy · 2015-07-09T18:38:54Z

👍 just remove the class expression label

JsonFreeman · 2015-07-09T18:47:23Z

@yuit Do you still need to do all these things now that you fixed the binder?

yuit · 2015-07-10T00:23:05Z

@JsonFreeman I make a change so that the service layer will get name through getDeclaredName which I prefer over using symbol.getName(). This makes completion list more consistent in the case of export default in that we won't show unicode escapes sequence in completion list at all. This will be some what consistent with other feature which show unicode escape sequence in its original declared form.

JsonFreeman · 2015-07-10T00:38:48Z

Great! Thanks Yui!

JsonFreeman · 2015-07-10T00:38:50Z

👍

yuit · 2015-07-10T15:42:09Z

@JsonFreeman Thank you for the good discussion we had 😄

… completionListWithLocalName

Completion list show correct entry for function expression and class expression

Yui T added 4 commits June 25, 2015 17:39

Add name of function expression into completion list

aab2100

Add test for completion in function expression

9c9e298

Add test for completion in class expression

34489fa

Support completion in named class expression and named function expre…

5467e1d

…ssion

msftclas added the cla-not-required label Jun 26, 2015

DanielRosenwasser reviewed Jun 26, 2015
View reviewed changes

Add todo comment to use getDeclaredName

45182b8

mhegazy reviewed Jun 26, 2015
View reviewed changes

Yui T added 8 commits June 29, 2015 16:04

Merge branch 'master' into completionListWithLocalName

553085f

Merge branch 'master' into completionListWithLocalName

389e446

Add comment to clarify why we don't use copySymbol

744f640

Address CR: Use getDeclaredName and getDeclarationOfKind

514d054

Fix rename for class expression

605ab0b

Clean up stripQuote and add comments

7747ad4

Update verification function to be able to test that the only symbol …

3079607

…with certain kind, document, and text is the completion list

Address code review

2e0c390

DanielRosenwasser reviewed Jul 1, 2015
View reviewed changes

Yui T added 4 commits July 1, 2015 18:45

Address code review

06c9876

Fix comments

6da98ce

Merge branch 'master' into completionListWithLocalName

7c52aaa

Conflicts: src/compiler/checker.ts

Address code review

b01e4d8

JsonFreeman reviewed Jul 6, 2015
View reviewed changes

Comment thread src/services/utilities.ts

JsonFreeman Jul 6, 2015

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

Nice, that is cute :)

Yui T added 4 commits July 6, 2015 17:03

Address code review, handle type parameter in completion list inside …

f4cd1ac

…class expression

Fix comment

0148915

Merge branch 'master' into completionListWithLocalName

872fdcf

Conflicts: tests/cases/fourslash/renameLocationsForClassExpression01.ts

Use symbol.getName for classExpression and functionExpression since i…

b5b1b7b

…t now correctly represent declared-name.

yuit mentioned this pull request Jul 8, 2015

Semantic LS fails in several different ways when dotting off of class expression name #3077

Closed

mhegazy reviewed Jul 9, 2015
View reviewed changes

Yui T added 2 commits July 9, 2015 16:34

Address code review

d0b8002

Use getDecalredName in service layer instead of symbol.getName

41bedd2

Yui T added 2 commits July 10, 2015 10:15

Add test for using unicode escape as function name

d0d1ee9

Merge branch 'master' of https://github.com/Microsoft/TypeScript into…

f16f9d1

… completionListWithLocalName

yuit added a commit that referenced this pull request Jul 10, 2015

Merge pull request #3643 from Microsoft/completionListWithLocalName

e72d0d8

Completion list show correct entry for function expression and class expression

yuit merged commit e72d0d8 into master Jul 10, 2015

yuit deleted the completionListWithLocalName branch July 10, 2015 20:02

microsoft locked and limited conversation to collaborators Jun 19, 2018

Uh oh!

Conversation

yuit commented Jun 26, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielRosenwasser commented Jun 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhegazy commented Jul 9, 2015

Uh oh!

JsonFreeman commented Jul 9, 2015

Uh oh!

yuit commented Jul 10, 2015

Uh oh!

JsonFreeman commented Jul 10, 2015

Uh oh!

JsonFreeman commented Jul 10, 2015

Uh oh!

yuit commented Jul 10, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants