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 Enum#to_s for flag enums to join with pipe #4005

Merged
merged 1 commit into from
Feb 9, 2017

Conversation

makenowjust
Copy link
Contributor

See #3961

@bcardiff
Copy link
Member

bcardiff commented Feb 6, 2017

This overrides #3961, right? I don't know why there were commas in the first and i didn't stop in questioning them to avoid the ambiguity the parens solved.

@bcardiff
Copy link
Member

bcardiff commented Feb 6, 2017

I've just read the 3961 thread. Unless something comes up i would also go with pipes :-)

@sdogruyol
Copy link
Member

Pipe wins 💯

@makenowjust
Copy link
Contributor Author

NOTE: Comma separation is corresponding to Enum.flag macro argument. It may be important.

@ysbaddaden
Copy link
Contributor

Good note. I believe Enum.flag to a macro to quickly a flags Enum, I'm not sure it should dictate an Enum representation.

@asterite
Copy link
Member

asterite commented Feb 8, 2017

Pipes are good for to_s. I originally copied the comma from to_s from how C# shows these enums, but using pipes is better.

@bcardiff bcardiff merged commit 104e2e7 into crystal-lang:master Feb 9, 2017
@bcardiff bcardiff added this to the Next milestone Feb 9, 2017
@makenowjust makenowjust deleted the fix/enum/pipe-to-s branch February 9, 2017 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants