b4n requested changes on this pull request.
I'd think `make distcheck` should be failing without cmark in the environment
@@ -1,2 +1 @@
-/peg-markdown/markdown_parser.c -/peg-markdown/peg-0.1.9/leg +
you could merely remove this file I believe
@@ -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.
@@ -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)
@@ -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_SUBST`ituing the macros and calling `AC_CONFIG_FILES([cmark/cmark_version.h])`), but you can do that with a `sed` call easily, something like this:
```makefile 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`](https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Particu...) 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.
@@ -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…
@@ -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])`
@@ -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`).
+#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_DEFINE`s `HAVE___BUILTIN_EXPECT`. I'm not sure how you check for this, but an `AC_TRY_COMPILE` would do I guess.
+#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
+#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
+#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`
- #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.
- 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*.