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

Added c++ version of the name generator #2

Merged
merged 2 commits into from Sep 23, 2015

Conversation

@Kronuz
Copy link
Contributor

commented Sep 21, 2015

Requires C++11

@Kronuz Kronuz force-pushed the Kronuz:master branch from 26b097b to 7223ccd Sep 21, 2015

@skeeto

This comment has been minimized.

Copy link
Owner

commented Sep 22, 2015

This is very interesting, thanks! It looks like you ported the
JavaScript version to C++.

I had trouble building it on my system (Debian Jessie, g++ 4.9),
particularly because I was getting a crazy compiler template error
message. After some investigation, I'm pretty sure Generator::symbols is
ill-defined because vector isn't permitted in C++. Vector
elements must be MoveAssignable (or does C++11 change this?). Removing
the const fixed the build. I also had to include to get
towupper().

I see the MIT license, but would you consider dedicating your
implementation to the public domain? The other three implementations are
public domain, per the UNLICENSE file in the repository root.

@Kronuz Kronuz force-pushed the Kronuz:master branch 3 times, most recently from ae0cbd1 to 7aba089 Sep 22, 2015

@Kronuz Kronuz force-pushed the Kronuz:master branch from 7aba089 to 8c8f324 Sep 22, 2015

@Kronuz

This comment has been minimized.

Copy link
Contributor Author

commented Sep 22, 2015

Yes, this is a port from JavaScript (also adds Collapse Triple option). I've updated the code to fix it in gcc (now it works in clang and gcc).

Regarding the license, I've also changed that to Public Domain in the headers.

@Kronuz Kronuz force-pushed the Kronuz:master branch 2 times, most recently from 966f6f5 to de21696 Sep 22, 2015

@Kronuz Kronuz force-pushed the Kronuz:master branch from de21696 to a5dee22 Sep 22, 2015

@skeeto skeeto merged commit a5dee22 into skeeto:master Sep 23, 2015

@skeeto

This comment has been minimized.

Copy link
Owner

commented Sep 23, 2015

Perfect, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.