-
-
Notifications
You must be signed in to change notification settings - Fork 702
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
Added description to EnumMember documentation to get name of value #7003
Conversation
Thanks for your pull request and interest in making D better, @Zevenberge! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + phobos#7003" |
@@ -4072,6 +4072,8 @@ Params: | |||
Returns: | |||
Static tuple composed of the members of the enumerated type `E`. | |||
The members are arranged in the same order as declared in `E`. | |||
The name of the enum can be found by querying the compiler for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You already have the name, otherwise case
wouldn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, we don't: https://run.dlang.io/is/FGGmOH
EnumMembers
returns an AliasSeq of values. E.g. members[i].stringof
returns cast(FooEnum)0
. I might be missing something as I'm no thorough expert.
On the other hand, if we can make it such that we can return the name more easily, then that has my preference as well. But I think that will be a breaking change, as members with the same value are no longer unique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently std.conv: to!string also works to retrieve the member as a string in this example. (It makes me wonder why there is a unittest for the identifier, but that is out of scope for improving the documentation.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import std.conv;
enum Enum {foo = 0, bar = 1, baz = 0}
static foreach(i, member; EnumMembers!Enum)
{
pragma(msg, to!string(member) ~ " " ~ __traits(identifier, EnumMembers!Enum[i]));
}
prints
foo foo
bar bar
foo baz
|
eeda79b
to
2fecc1b
Compare
2fecc1b
to
db843f1
Compare
Alright, this documentation should fit the implementation (and clarifies the difference between __traits and to!string). |
Explicitly stated that the enum name can be retrieved with __traits. There was a unittest for this, but it was not stated in the documentation.
I've added an example to generate a switch-statement with static foreach. Furthermore, I removed the generate example from the documentation, as (in my eyes) it is basically a duplicate of the square root example.
Suggestions for improvement are appreciated.