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

Improve CMake project #180

Merged
merged 1 commit into from Dec 14, 2017
Merged

Improve CMake project #180

merged 1 commit into from Dec 14, 2017

Conversation

podsvirov
Copy link
Contributor

Try it:

> git clone -b cmake https://github.com/podsvirov/discount.git
> cd discount
> cmake cmake/
> make

@Orc
Copy link
Owner

Orc commented Dec 13, 2017

I can't pull that as is, because it #defines strcase?cmp as str?cmp on non-windows machines. Surely cmake has a way to test for the existance of strcasecmp and strncasecmp?

@Orc
Copy link
Owner

Orc commented Dec 13, 2017

And, also,

orc@pie(discount)> cmake
-ksh: cmake: not found

There's no way for me to test cmake environments, because I don't have cmake installed on any of my build & test machines. cmake, like plan9 & other windows-specific build configurations, are taken on faith after checking for unpleasant surprises.

@podsvirov
Copy link
Contributor Author

What OS on your build & test machines?

@Orc
Copy link
Owner

Orc commented Dec 13, 2017

Darwin, FreeBSD, Debian, and Centos. Cmake free by preference.

@podsvirov
Copy link
Contributor Author

CMake can check the existance of strcasecmp and strncasecmp. I work on this now.

@podsvirov
Copy link
Contributor Author

On my server with Debian to install cmake I simple type:

> sudo apt-get install cmake

@Orc
Copy link
Owner

Orc commented Dec 13, 2017

If I wanted cmake I'm sure I could install it on everything (except maybe the FreeBSD machine.)

@podsvirov podsvirov changed the title Improve CMake project to build on Linux Improve CMake project Dec 13, 2017
@podsvirov
Copy link
Contributor Author

@Orc, I add more system checks. Please review again.

@Orc
Copy link
Owner

Orc commented Dec 13, 2017

One thing I noticed in the latest patch; you're #define'ing HAVE_{symbol} to 0 for a handful of things if they don't exist; you shouldn't do that -- I try to use #if for all of the HAVE_{symbol} checks, but there are a handful of places where it's still a #ifdef & even when I fix them I want to reserve any symbol definition for symbols that exist just in case I run into a cpp somewhere that pukes on either #if with a nonexistant symbol or even #if all by itself.

Added more system checks to build
on different platforms.
@podsvirov
Copy link
Contributor Author

Ok. Fixed. Please review.

@podsvirov
Copy link
Contributor Author

Funny bonus. With these changes I was able to build and test the markdown.js via emcc compiler (Emscripten project).
And from emsdk command prompt on Windows I type (and it's work):

> type syntex.text | node markdown.js > syntex.html

@Orc Orc merged commit 80e1ad5 into Orc:master Dec 14, 2017
@Orc
Copy link
Owner

Orc commented Dec 14, 2017

It's amusing that the js crosscompiler works as well. I'll have to spin up a vm and take a look at it.

@podsvirov
Copy link
Contributor Author

I think that vm it's not necessary. This is a cross-platform project. Installation instructions for all supported platforms are available on the site.

@Orc
Copy link
Owner

Orc commented Dec 15, 2017

It's a lot easier to spin up a vm than to install an OS onto physical hardware; most (maybe all?) of the vm supervisors out there implement machines with middle of the road (and well-supported) hardware, plus there's no need to mess around with physical install media.

@podsvirov
Copy link
Contributor Author

Of course, do as you please. I spoke more about the operating system.

@podsvirov
Copy link
Contributor Author

The goal can be not only a clean JavaScript script, but also a WebAssembly module.
I plan to try your library on the client side (in the browser) also expect it to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants