Skip to content

Handle const modifier in getOccurrences#1333

Merged
mhegazy merged 4 commits into
masterfrom
getOccurancesAssert
Dec 3, 2014
Merged

Handle const modifier in getOccurrences#1333
mhegazy merged 4 commits into
masterfrom
getOccurancesAssert

Conversation

@mhegazy
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy commented Dec 2, 2014

Fix for #1331. Const is not handled as a modifier, so add the handling for it.

Comment thread src/services/services.ts Outdated
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.

Are there any contexts where a const modifier can appear, but where it is not allowed? What if I have the following?

class C {
    const foo;
    const bar;
}

Currently we don't highlight modifiers when they appear in an incorrect context.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that is not legal syntax.

@DanielRosenwasser DanielRosenwasser changed the title Handel modifier const in getOccurrances Handle const modifier in getOccurrences Dec 2, 2014
Comment thread src/services/services.ts Outdated
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.

Maybe we should add an else clause and bail out? Is it better to crash and hope to catch in dogfooding, or let it gracefully do nothing?

@sheetalkamat
Copy link
Copy Markdown
Member

👍

Comment thread src/services/services.ts Outdated
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.

unsupported

@DanielRosenwasser
Copy link
Copy Markdown
Member

Other than my last comment, 👍.

mhegazy added a commit that referenced this pull request Dec 3, 2014
Handle const modifier in getOccurrences
@mhegazy mhegazy merged commit c075fe1 into master Dec 3, 2014
@mhegazy mhegazy deleted the getOccurancesAssert branch December 3, 2014 05:08
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 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.

3 participants