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

Fix functions returning references and return type declarations #368

Merged
merged 5 commits into from Nov 6, 2019

Conversation

@KapitanOczywisty
Copy link
Contributor

KapitanOczywisty commented Aug 29, 2019

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Functions returning references are now correctly colored #343
Return types are now correctly identified and colored #367
Return types now are allowed to include namespaces
Test spec for above

Alternate Designs

Class in return type could be further processed to separate individual parts like namespace, class name. Exactly how function parameters are done, but this is beyond my patience.

Benefits

Correct colors

Possible Drawbacks

I hope none

Applicable Issues

fix #343 , fix #367 , fix #320

@KapitanOczywisty

This comment has been minimized.

Copy link
Contributor Author

KapitanOczywisty commented Aug 29, 2019

Also #320 partially fixed (look at Alternate Design)

@rsese

This comment has been minimized.

Copy link
Member

rsese commented Aug 29, 2019

Thanks @KapitanOczywisty! Can you add tests so your fixes aren't broken by future changes?

Though a maintainer might take a look here, if you need help adding tests the best place to get help with test issues is probably the Atom Slack. There are a bunch of helpful folks there that should be willing to help.

@KapitanOczywisty

This comment has been minimized.

Copy link
Contributor Author

KapitanOczywisty commented Aug 31, 2019

@rsese tests ready :)

@KapitanOczywisty KapitanOczywisty force-pushed the KapitanOczywisty:master branch from 97fe304 to 651ed94 Aug 31, 2019
@rsese rsese added triaged and removed needs-testing labels Sep 3, 2019
@darangi darangi self-assigned this Nov 6, 2019
@darangi darangi merged commit 4710c91 into atom:master Nov 6, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.