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

Cannot lookup symbol of nested function in method #6828

Closed
dbaeumer opened this issue Feb 2, 2016 · 10 comments
Closed

Cannot lookup symbol of nested function in method #6828

dbaeumer opened this issue Feb 2, 2016 · 10 comments
Labels
API Relates to the public API for TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Feb 2, 2016

From @tinganho on January 31, 2016 17:22

I cannot get a symbol of nested function in method.

class A {
    b() {
       function test() {

       }
    }
}

I think the symbol test should show up when I do CMD + SHIFT + O. Since it works with nested functions:

function test1 () {
    function test2() {

    }
}

I can get test2 above.

Copied from original issue: microsoft/vscode#2584

@dbaeumer
Copy link
Member Author

dbaeumer commented Feb 2, 2016

From @aeschli on February 1, 2016 10:35

@tinganho I assume this is TypeScript?

@dbaeumer
Copy link
Member Author

dbaeumer commented Feb 2, 2016

From @tinganho on February 1, 2016 10:35

Yes.

@dbaeumer
Copy link
Member Author

dbaeumer commented Feb 2, 2016

Moving to the TS team.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 2, 2016

this is by desing to limit noise in the list. functions are supported to allow for JS coding patterns using nested functions. which does not seem to be that common with classes.

@mhegazy mhegazy closed this as completed Feb 2, 2016
@mhegazy mhegazy added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Feb 2, 2016
@dbaeumer
Copy link
Member Author

dbaeumer commented Feb 3, 2016

@mhegazy I think the tsserver nor the language service should make that 'design' decision. It should report the full structure. If a user interface does want to limit the noise we can either filter on the UI side or instruct the tsserver with an option when running the request.

@tinganho
Copy link
Contributor

tinganho commented Feb 3, 2016

this is by design to limit noise in the list.

@mhegazy I think I disagree here. It adds as much noise as if, if I would use a private method. Though the downside with a private method is that they cannot access local variables.

I have found it quite useful to nest functions instead of declaring private methods. Since they are declared inside the methods which they are used. And most of my private methods is only used by one other method, I think it is more intuitive to nest the functions instead. Though, for very large methods I got a lot of nested functions. And not having the ability to find them by symbol, makes my productivity go down. And I also think something is broken every time I don't find something auto completing or not being able to find something by symbol.

@tinganho
Copy link
Contributor

tinganho commented Feb 3, 2016

Just to clarify this is the pattern I use:

Instead of this:

if (true) {
    a += 1;
    b += 1;
    c += 1;
    d += 1;
}

I can refactor it as:

if (true) {
   incrementABCD();
}

function incrementABCD() {
    a += 1;
    b += 1;
    c += 1;
    d += 1;
}

I've learnt this pattern through your source code 😉 . Though, I don't want to go pure functional since I think it lacks some typing that classes got.

@mhegazy mhegazy added Suggestion An idea for TypeScript Help Wanted You can do this API Relates to the public API for TypeScript and removed By Design Deprecated - use "Working as Intended" or "Design Limitation" instead labels Feb 3, 2016
@mhegazy mhegazy added this to the Community milestone Feb 3, 2016
@mhegazy mhegazy reopened this Feb 3, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Feb 3, 2016

PRs welcomed.

@DanielRosenwasser
Copy link
Member

Also related is #4481.

@DanielRosenwasser
Copy link
Member

Should be fixed - thanks @tinganho!

@DanielRosenwasser DanielRosenwasser added the Fixed A PR has been merged for this issue label Mar 26, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Relates to the public API for TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants