Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Highlight Abstract occurrences#3846

Merged
DanielRosenwasser merged 4 commits into
microsoft:mastermicrosoft/TypeScript:masterfrom
DickvdBrink:abstract-occurrencesDickvdBrink/TypeScript:abstract-occurrencesCopy head branch name to clipboard
Jul 14, 2015
Merged

Highlight Abstract occurrences#3846
DanielRosenwasser merged 4 commits into
microsoft:mastermicrosoft/TypeScript:masterfrom
DickvdBrink:abstract-occurrencesDickvdBrink/TypeScript:abstract-occurrencesCopy head branch name to clipboard

Conversation

@DickvdBrink

Copy link
Copy Markdown
Contributor

My attempt at highlighting abstract keyword (#3840)

Can add more tests if required.
Feel free to close if you want this solved differently.

Comment thread src/services/services.ts Outdated

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.

nit: else on a new line.

@mhegazy

mhegazy commented Jul 13, 2015

Copy link
Copy Markdown
Contributor

👍

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.

These are testing the same thing. I think you mean ClassExpression? But then you can't make a class expression abstract. Please add a test for that.

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.

Don't think these are the same or am I wrong here? On members of a class the container.kind is the ClassDeclaration, on the abstract keyword of the class the container is actually a source file so that is why I added the declaration.kind check.

Will do some test for ClassExpressions. Thanks for the feedback!

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.

Btw: This is what you meant with ClassExpression right?

function yolo() {
    abstract class test {
        foo: number;
    }
}

I get an error that Modifiers cannot appear here

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.

Nit: Space after for

@DanielRosenwasser

Copy link
Copy Markdown
Member

Can we

  1. Rename the current test to getOccurrencesAbstract01.ts
  2. Add an abstract property prop1 to the class of interest in getOccurrencesAbstract01.ts
  3. Add an abstract method named abstract() to the class of interest in getOccurrencesAbstract01.ts
  4. Make a test where you have abstract methods in a non-abstract class named getOccurrencesAbstract02.ts
  5. Make a test where you have abstract methods in a class expression (basically the last test, but add let C = to the beginning of the declaration), named getOccurrencesAbstract02.ts

@DickvdBrink

Copy link
Copy Markdown
Contributor Author

Done, I think I did it the way you wanted :)

@DanielRosenwasser

Copy link
Copy Markdown
Member

Well I made a mistake in my instructions - I said getOccurrencesAbstract02 for the last test instead of getOccurrencesAbstract03 but I think that should be fine. Thanks for the PR @DickvdBrink!

DanielRosenwasser added a commit that referenced this pull request Jul 14, 2015
@DanielRosenwasser DanielRosenwasser merged commit c5aa3ad into microsoft:master Jul 14, 2015
@mhegazy

mhegazy commented Jul 14, 2015

Copy link
Copy Markdown
Contributor

Thanks!

@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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.