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)/[email protected] > $@
```
Don't forget to check for
[`AC_PROG_SED`](https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Particular-Programs.html#index-AC_005fPROG_005fSED-293)
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*.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/747#pullrequestreview-123218588
