Added CMake project and fixed compiling under MSVC.

ooc-lang / greg

:ram: a recursive-descent parser generator for PEG

93 stars 30 forks source link

Added CMake project and fixed compiling under MSVC. #16

Closed kainjow closed 10 years ago

nddrylliog commented 10 years ago

Hey @kainjow, just tested your PR on Mac, Linux and mingw-w64, seems to build cleanly.

A few things to fix in your PR:

You must change greg.g, not greg.c, since the latter is regenerated from greg.g.
I see the point of wingetopt, and it's a good idea, but I don't think it's acceptable as a submodule of greg only for MSVC compilation - document somewhere that it is required to build on MSVC, but don't impose a dependency where it builds fine without on the three main targets.
By the way, since your wingetopt repo takes code from mingw, please license it correctly (by committing a 'LICENSE' file to the repository - I believe it is BSD, but I might be mistaken) and including a list of original authors in your README. Short of that, I wouldn't feel comfortable recommending usage of your repo in greg's README :)

When that is done I see no objections in merging that.

kainjow commented 10 years ago

Hi @nddrylliog,

Oops, I'll make the changes to greg.g.

The winoptget code appears to be in public domain according to the headers. You think it still needs a LICENSE file? http://sourceforge.net/apps/trac/mingw-w64/browser/trunk/mingw-w64-headers/crt/getopt.h It looks like it was BSD but changed to Public Domain: http://sourceforge.net/apps/trac/mingw-w64/changeset/1794

kainjow commented 10 years ago

Ah, it's only the header that's Public Domain, but the source is BSD. Ok, I'll add the license file.

Should I just list MinGW as the authors? I'm not too familiar with making those.

nddrylliog commented 10 years ago

@kainjow The authors are listed in the source files - in this case, Todd Miller and the NetBSD Foundation

For big lists of authors, I like to have an 'AUTHORS.md' file, like so: https://github.com/nddrylliog/rock/blob/master/AUTHORS.md

When it's rather short, a list in the README is enough: https://github.com/fredreichbier/ooc-lua

nddrylliog commented 10 years ago

For greg.g, you can verify that your changes are correct by doing 'make grammar' and doing 'make' again (you should probably add equivalents for that to the CMakeLists file - I've never written one myself so I wouldn't know where to start). It'll regenerate greg.c using greg. Your pull request should include modifications both to greg.g and greg.c and the state of the repo should be clean when running 'make && make grammar' on a fresh clone :) (which should be trivial).

kainjow commented 10 years ago

Ok think I fixed everything up as you suggested.

Example CMake usage:

mkdir -p build
cd build
cmake ..
make greg grammar

"greg" and "grammar" will also be targets for IDEs such as Xcode or Visual Studio if those generators are used.

nddrylliog commented 10 years ago

Looks nice. Are we sure using "rb" and "wb" rather than "r" and "w" doesn't have any adverse effects on non-mingw platforms? I suppose it might change line ending behavior. In ooc's grammar we have stuff to handle '\r', '\n', and "\r\n" already but others might not react as well to the change.

I'm on vacation until next Wednesday, so ping me then again so I can take a closer look and merge it all. Thanks!

kainjow commented 10 years ago

"b" is optional as far as I know everywhere except MSVC, which defaults to text mode.

nddrylliog commented 10 years ago

@kainjow Finally got time to review & merge. Thank!