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

Change iftype to use elseif instead of elseiftype as next keyword. #1905

Merged
merged 1 commit into from
May 13, 2017

Conversation

jemc
Copy link
Member

@jemc jemc commented May 13, 2017

This PR changes one of the keywords used in the new iftype syntax introduced in #1855.

Specifically, it changes the use of elseiftype to elseif. This is more consistent with ifdef, which uses elseif and not elseifdef.

The detail of what keyword to use for this wasn't included in the RFC, and I doubt very many people are using this feature yet given that the plan was to not formally announce it until the rest of the RFC was complete, so it seems like fair game to change.

Also, there were no tests using elseiftype, so I added a test using elseif.

@jemc jemc requested a review from Praetonus May 13, 2017 01:39
@jemc jemc self-assigned this May 13, 2017
@jemc
Copy link
Member Author

jemc commented May 13, 2017

Other than consistency, this also gives a better visual alignment of the conditions:

Before:

iftype A <: T1 then foo()
elseiftype A <: T2 then bar()
else baz()
end

After:

iftype A <: T1 then foo()
elseif A <: T2 then bar()
else baz()
end

@jemc jemc added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label May 13, 2017
@Praetonus Praetonus merged commit 1e33f5b into master May 13, 2017
ponylang-main added a commit that referenced this pull request May 13, 2017
@Praetonus Praetonus deleted the refactor/iftype-elseif branch May 13, 2017 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants