Skip to content

Adds quick info on modifiers and declaration keywords #3189

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

Closed
wants to merge 3 commits into from

Conversation

tinganho
Copy link
Contributor

This PR implements #2736. I just implemented the basic display part for function right now.

function FunctionName() {}
function () {}

I can rework this PR to add/remove/change the display parts.

It also add some contextual quick info on anonymous functions.

@tinganho tinganho force-pushed the modifierQuickInfo branch 3 times, most recently from d0c2e17 to 988d2d3 Compare May 17, 2015 13:02
@tinganho tinganho force-pushed the modifierQuickInfo branch from 988d2d3 to 0283e79 Compare May 17, 2015 13:11
@@ -1377,6 +1379,7 @@ module ts {
/* @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol
valueDeclaration?: Declaration; // First value declaration of the symbol
/* @internal */ constEnumOnlyModule?: boolean; // True if module contains only const enums or other modules with only const enums
isAnonymous?: boolean; // True if declaration is anonymous
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need this? this information can be easily inferred from other sources, like the name, and the node flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mhegazy I was trying a lot of different ways to pinpoint anonymous declarations. I just thought this was easier. But you're right, we shouldn't add something just to serve one very specific use case.

It would be good if you could help me out on the below section. Where I use this property to add a name of a declaration/symbol.

@CyrusNajmabadi
Copy link
Contributor

What is C#'s behavior? Do they also give quick info on modifiers? If not, i don't think we should be behaving differently.

@@ -234,7 +234,7 @@ module ts.formatting {
// then kind needs to be fixed. This might happen in cases
// when parser interprets token differently, i.e keyword treated as identifier
function fixTokenKind(tokenInfo: TokenInfo, container: Node): TokenInfo {
if (isToken(container) && tokenInfo.token.kind !== container.kind) {
if (isReservedWord(container) && tokenInfo.token.kind !== container.kind) {
tokenInfo.token.kind = container.kind;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want Token here.. @vladima?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just renamed isToken to isReservedWord. I thought isToken where to ambitious. And when I look at the implementation – it only contains a check for reserved words.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 19, 2015

@tinganho are you still looking into this?

@tinganho
Copy link
Contributor Author

I have been a little bit busy lately. I will update the PR in next week.

On Fri, Jun 19, 2015 at 8:18 AM, Mohamed Hegazy [email protected]
wrote:

@tinganho https://github.com/tinganho are you still looking into this?


Reply to this email directly or view it on GitHub
#3189 (comment)
.

Sincerely,

Tingan Ho
@tingan87 https://twitter.com/tingan87

@tinganho
Copy link
Contributor Author

@mhegazy I think I will close this PR. It's a little bit outdated.

@tinganho tinganho closed this Jun 25, 2015
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants