@b4n requested changes on this pull request.

I'd think make distcheck should be failing without cmark in the environment

In markdown/.gitignore:

> @@ -1,2 +1 @@
-/peg-markdown/markdown_parser.c
-/peg-markdown/peg-0.1.9/leg
+

you could merely remove this file I believe

In markdown/Makefile.am:

> @@ -1,11 +1,5 @@
 include $(top_srcdir)/build/vars.auxfiles.mk
 
-SUBDIRS =
-
-if MARKDOWN_PEG_MARKDOWN
-SUBDIRS += peg-markdown
-endif
-
-SUBDIRS += src docs
+SUBDIRS = cmark src docs

I would rather not recurse inside cmark if it isn't enabled (as it was done for peg-markdown). But either works, so if you prefer this it's not a problem.

In markdown/cmark/Makefile.am:

> @@ -0,0 +1,43 @@
+if MD_BUILTIN_CMARK
+
+noinst_LTLIBRARIES = libcmark.la
+
+libcmark_la_SOURCES = \
+	blocks.c \
+	buffer.c \
+	buffer.h \
+	chunk.h \
+	cmark.c \
+	cmark_ctype.c \
+	cmark_ctype.h \
+	cmark.h \

you miss at least cmark_version.h, so it will not be included in the tarball (as it's not listed anywhere I can see). Check witt the distcheck make target, and obviously on an environment lacking the cmark library (or explicitly enabling the builtin copy)

In markdown/cmark/Makefile.am:

> @@ -0,0 +1,43 @@
+if MD_BUILTIN_CMARK
+
+noinst_LTLIBRARIES = libcmark.la
+
+libcmark_la_SOURCES = \
+	blocks.c \
+	buffer.c \
+	buffer.h \
+	chunk.h \
+	cmark.c \
+	cmark_ctype.c \
+	cmark_ctype.h \
+	cmark.h \

you could generate the cmark_version.h automatically from a cmark_version.h.in fairly easily. Unfortunately the placeholder names are not namespaced properly (PROJECT_) so it wouldn't be too clean to replace them with the proper Autotools feature (which is sad, as it makes it super trivial, a matter of AC_SUBSTituing the macros and calling AC_CONFIG_FILES([cmark/cmark_version.h])), but you can do that with a sed call easily, something like this:

CMARK_VERSION_MAJOR=0
CMARK_VERSION_MINOR=28
CMARK_VERSION_PATCH=3

CLEANFILES = cmark_version.h
EXTRA_DIST = cmark_version.h.in

cmark_version.h: cmark_version.h.in Makefile
	$(SED) -e 's,[@]PROJECT_VERSION_MAJOR[@],$(CMARK_VERSION_MAJOR),g' \
	       -e 's,[@]PROJECT_VERSION_MINOR[@],$(CMARK_VERSION_MINOR),g' \
	       -e 's,[@]PROJECT_VERSION_PATCH[@],$(CMARK_VERSION_PATCH),g' \ 
	       < $(srcdir)/$@.in > $@

Don't forget to check for AC_PROG_SED in markdown.m4 when the plugin is enabled (I would think it's already done somewhere (libtool?), but doing it for the plugin is safe(r))

Likewise, you could generate cmark_export.h to your needs (and if it has to include stdbool.h for a hack of some kind, so be it).

That's just suggestions for easing the update of the embedded library by minimizing and hopefully removing any difference. I would think their cmark_version.h file is auto-generated (or at least processed) anyway, so that should not create additional diversion.

In markdown/cmark/config.h.in:

> @@ -0,0 +1,76 @@
+#ifndef CMARK_CONFIG_H
+#define CMARK_CONFIG_H

this file seems useless in the current setup. However, some of its content could be moved to Autotools…

In markdown/cmark/config.h.in:

> @@ -0,0 +1,76 @@
+#ifndef CMARK_CONFIG_H
+#define CMARK_CONFIG_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#cmakedefine HAVE_STDBOOL_H

AC_CHECK_HEADERS([stdbool.h])

In markdown/cmark/config.h.in:

> @@ -0,0 +1,76 @@
+#ifndef CMARK_CONFIG_H
+#define CMARK_CONFIG_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#cmakedefine HAVE_STDBOOL_H
+
+#ifdef HAVE_STDBOOL_H
+  #include <stdbool.h>
+#elif !defined(__cplusplus)
+  typedef char bool;
+#endif

Trickier, because Autotools doesn't really have a clean way of force-including stuff, and this file is a mere config.h… one way could be having a second config.h just for cmark, but I'm not sure how to manage this with stock Autotools, short of manually generating that one (e.g. having a template and performing the replacements or using AC_SUBST/AC_CONFIG_FILES).

In markdown/cmark/config.h.in:

> +#ifndef CMARK_CONFIG_H
+#define CMARK_CONFIG_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#cmakedefine HAVE_STDBOOL_H
+
+#ifdef HAVE_STDBOOL_H
+  #include <stdbool.h>
+#elif !defined(__cplusplus)
+  typedef char bool;
+#endif
+
+#cmakedefine HAVE___BUILTIN_EXPECT

You might want to add a check for __builtin_expect() that AC_DEFINEs HAVE___BUILTIN_EXPECT. I'm not sure how you check for this, but an AC_TRY_COMPILE would do I guess.

In markdown/cmark/config.h.in:

> +
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#cmakedefine HAVE_STDBOOL_H
+
+#ifdef HAVE_STDBOOL_H
+  #include <stdbool.h>
+#elif !defined(__cplusplus)
+  typedef char bool;
+#endif
+
+#cmakedefine HAVE___BUILTIN_EXPECT
+
+#cmakedefine HAVE___ATTRIBUTE__

Same

In markdown/cmark/config.h.in:

> +
+#ifdef HAVE_STDBOOL_H
+  #include <stdbool.h>
+#elif !defined(__cplusplus)
+  typedef char bool;
+#endif
+
+#cmakedefine HAVE___BUILTIN_EXPECT
+
+#cmakedefine HAVE___ATTRIBUTE__
+
+#ifdef HAVE___ATTRIBUTE__
+  #define CMARK_ATTRIBUTE(list) __attribute__ (list)
+#else
+  #define CMARK_ATTRIBUTE(list)
+#endif

Or instead of defining HAVE___ATTRIBUTE__, just AC_DEFINE CMARK_ATTRIBUTE as appropriate. Though, I'm not sure it's possible to define a macro taking arguments… at worse, just define a custom CPPFLAG playing with -D I guess

In markdown/cmark/config.h.in:

> +
+#cmakedefine HAVE___ATTRIBUTE__
+
+#ifdef HAVE___ATTRIBUTE__
+  #define CMARK_ATTRIBUTE(list) __attribute__ (list)
+#else
+  #define CMARK_ATTRIBUTE(list)
+#endif
+
+#ifndef CMARK_INLINE
+  #if defined(_MSC_VER) && !defined(__cplusplus)
+    #define CMARK_INLINE __inline
+  #else
+    #define CMARK_INLINE inline
+  #endif
+#endif

similar here, you could check for availability and AC_DEFINE CMARK_INLINE, or use -D

In markdown/cmark/config.h.in:

> +  #define CMARK_ATTRIBUTE(list) __attribute__ (list)
+#else
+  #define CMARK_ATTRIBUTE(list)
+#endif
+
+#ifndef CMARK_INLINE
+  #if defined(_MSC_VER) && !defined(__cplusplus)
+    #define CMARK_INLINE __inline
+  #else
+    #define CMARK_INLINE inline
+  #endif
+#endif
+
+/* snprintf and vsnprintf fallbacks for MSVC before 2015,
+   due to Valentin Milea http://stackoverflow.com/questions/2915672/
+*/

I'm not sure if we support compiling with MSVC at all, but if it's the case and cmark requires properly behaving [v]snprintf() it might be important to have a similar thing.

In markdown/cmark/Makefile.am:

> +	node.c \
+	node.h \
+	parser.h \
+	references.c \
+	references.h \
+	render.c \
+	render.h \
+	scanners.c \
+	scanners.h \
+	utf8.c \
+	utf8.h \
+	xml.c
+
+EXTRA_DIST = \
+	case_fold_switch.inc \
+	entities.inc

you also miss scanners.re.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.