Markdown: replace Discount and PEG Markdown with CMark #747

codebrainz · 2018-05-24T08:23:59Z

Adds the CMark source into the plugin tree to be used when there is no libcmark package available using pkg-config. At present, my distro only ships the cmark utility but not the accompanying library or development packages, as well I do not believe msys2 provides a CMark package for Windows, so for now it will remain embedded as a backup, replacing the previously embedded PEG Markdown and the externally linked Discount markdown library.

The version embedded is v0.28.3. A few trivial changes were made to allow libcmark to compile without its original CMake infrastructure. Only cmark.h and cmark_version.h were modified.

TODO: create a patch so the simple changes to upstream CMark library can be mechanically applied to newer versions.

All future complaints/requests about supporting various Markdown extensions should be addressed directly to the CommonMark Project.

Fixes: #589, #651.

Adds the CMark source into the plugin tree to be used when there is no `libcmark` package available using `pkg-config`. At present, my distro only ships the `cmark` utility but not the accompanying library or development packages, as well I do not believe msys2 provides a CMark package for Windows, so for now it will remain embedded as a backup. The version embedded is v0.28.3, from commit 9f8ef820301951f36301c1a40d036cafeaa78619. A few trivial changes were made to allow libcmark to compile without its original CMake infrastructure. Only `cmark.h` and `cmark_version.h` were modified. TODO: create a patch so the simple changes to upstream CMark library can be mechanically applied to newer versions. All future complaints/requests about supporting various Markdown extensions should be addressed directly to the [CommonMark Project](http://commonmark.org/).

codebrainz · 2018-05-24T08:29:02Z

Also ping @evgeni, @sardemff7 et al. 😄

codebrainz · 2018-05-24T08:38:49Z

For anyone interested in reviewing the non-build-related code, the crux of it is here.

codebrainz · 2018-05-24T08:52:56Z

For future me, diffs from upstream:

@@ -1,10 +1,13 @@
 #ifndef CMARK_H
 #define CMARK_H
 
+#include <stdbool.h>
 #include <stdio.h>
-#include <cmark_export.h>
 #include <cmark_version.h>
 
+#define CMARK_EXPORT
+#define CMARK_INLINE inline
+
 #ifdef __cplusplus
 extern "C" {
 #endif

and

@@ -1,7 +1,7 @@
 #ifndef CMARK_VERSION_H
 #define CMARK_VERSION_H
 
-#define CMARK_VERSION ((@PROJECT_VERSION_MAJOR@ << 16) | (@PROJECT_VERSION_MINOR@ << 8)  | @PROJECT_VERSION_PATCH@)
-#define CMARK_VERSION_STRING "@PROJECT_VERSION_MAJOR@.@PROJECT_VERSION_MINOR@.@PROJECT_VERSION_PATCH@"
+#define CMARK_VERSION ((0 << 16) | (28 << 8)  | 3)
+#define CMARK_VERSION_STRING "0.28.3"
 
 #endif

evgeni · 2018-05-24T12:04:23Z

@codebrainz why did you ping me? :)
debian maintenance is done mostly by @hyperair these days - I am rather busy with other things.
but cmark is available in debian and i guess we would not really care what you embed as a fallback.

codebrainz · 2018-05-24T23:45:54Z

@evgeni sorry, I wasn't aware, I just wanted to get some people who know the project and about packaging and Autotools to take a look, since that's mostly what this PR affects. Sorry for the (now two) pings. I did see that a libcmark package was added very recently, but I just embedded it until it's widely available. Thanks for your time.

b4n

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

b4n · 2018-05-25T01:03:28Z

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

b4n · 2018-05-25T01:05:54Z

markdown/Makefile.am

-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.

I'm indifferent.

b4n · 2018-05-25T01:13:22Z

markdown/cmark/Makefile.am

+	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)

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.

Good call on the cmark_version.h missing. I tried to run make distcheck but it failed in what seemed like not my plugin's code (in util IIRC), will have to try again.

Will look at generating cmark_version.h, I was too lazy to bother as the changes I made are quite trivial to reproduce if/when I update the embedded version, but it is better to generate it. I briefly looked at cmark_export.h.in but I couldn't really figure out where it came from, and I can't read the awful cmake language.

About distcheck failure in util I don't know but could believe it, I'm afraid nobody tried it since util was introduced. I'll try to get a look into this when I can.

I briefly looked at cmark_export.h.in but I couldn't really figure out where it came from

Isn't the .in the source itself? Usually it's the suffix for files that will be processed, and the suffix is stripped from the output filename.

Sorry, I made a typo with the .in suffix, it's just cmark_export.h and I have no clue where it comes from, it's not in the sources, I can only suppose that it's entirely generated from CMake, but I couldn't stomach reading that horrid language.

b4n · 2018-05-25T02:00:29Z

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…

I don't think I want to care about anything in this file that I can trivially stub-out. Maybe I could just drop it, will have to look closer. Same for most of below comments on this file.

b4n · 2018-05-25T02:00:54Z

markdown/cmark/config.h.in

+extern "C" {
+#endif
+
+#cmakedefine HAVE_STDBOOL_H


AC_CHECK_HEADERS([stdbool.h])

b4n · 2018-05-25T02:05:19Z

markdown/cmark/config.h.in

+
+#cmakedefine HAVE___BUILTIN_EXPECT
+
+#cmakedefine HAVE___ATTRIBUTE__


b4n · 2018-05-25T02:07:33Z

markdown/cmark/config.h.in

+  #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

b4n · 2018-05-25T02:08:32Z

markdown/cmark/config.h.in

+  #else
+    #define CMARK_INLINE inline
+  #endif
+#endif


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

b4n · 2018-05-25T02:11:26Z

markdown/cmark/config.h.in

+
+/* 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.

b4n · 2018-05-25T02:19:48Z

markdown/cmark/Makefile.am

+
+EXTRA_DIST = \
+	case_fold_switch.inc \
+	entities.inc


you also miss scanners.re.

Oops, I meant to delete that file and just use the pre-generated code from upstream.

codebrainz · 2020-05-27T23:06:47Z

I've recently upgraded to Ubuntu 20.04 and I see that now libcmark is packaged separately, unfortunately it has a broken pkg-config file. I was also excited to see that there's a Github fork libcmark-gfm that features Github Flavoured Markdown, which would add support for some of the widely requested extensions (ex. tables), but again, unfortunately the pkg-config file for this one is entirely missing (it's upstream but not in the Ubuntu package).

Grrrrr....

codebrainz requested review from hyperair, dmaphy, b4n and frlan May 24, 2018 08:28

This was referenced May 24, 2018

Markdown: Support for backticks #592

Closed

CommonMark plugin #589

Closed

Markdown: Support tables extensions #184

Closed

Markdown: add support for webkit2gtk #746

Merged

b4n requested changes May 25, 2018

View reviewed changes

sergiomb2 mentioned this pull request May 31, 2018

Port both webhelper and markdown to webkit2gtk #677

Merged

elextr mentioned this pull request Jul 2, 2018

plugin markdown #757

Open

codebrainz mentioned this pull request Sep 10, 2018

markdown: in html, generated TOC, the links are dead #771

Closed

lpaulsen93 added enhancement markdown plugin/markdown labels Jun 29, 2019

ghost mentioned this pull request Jan 22, 2020

Support for commonmark markdown syntax #651

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Markdown: replace Discount and PEG Markdown with CMark #747

Markdown: replace Discount and PEG Markdown with CMark #747

codebrainz commented May 24, 2018 •

edited

Loading

codebrainz commented May 24, 2018

codebrainz commented May 24, 2018

codebrainz commented May 24, 2018

evgeni commented May 24, 2018 •

edited

Loading

codebrainz commented May 24, 2018

b4n left a comment

b4n May 25, 2018

codebrainz May 25, 2018

b4n May 25, 2018

codebrainz May 25, 2018

b4n May 25, 2018

b4n May 25, 2018

codebrainz May 25, 2018

b4n May 25, 2018

codebrainz May 25, 2018

b4n May 25, 2018

codebrainz May 25, 2018

b4n May 25, 2018

b4n May 25, 2018

b4n May 25, 2018

b4n May 25, 2018

b4n May 25, 2018

b4n May 25, 2018

codebrainz May 25, 2018

codebrainz commented May 27, 2020 •

edited

Loading

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

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


		#cmakedefine HAVE___BUILTIN_EXPECT

		#cmakedefine HAVE___ATTRIBUTE__

Markdown: replace Discount and PEG Markdown with CMark #747

Are you sure you want to change the base?

Markdown: replace Discount and PEG Markdown with CMark #747

Conversation

codebrainz commented May 24, 2018 • edited Loading

codebrainz commented May 24, 2018

codebrainz commented May 24, 2018

codebrainz commented May 24, 2018

evgeni commented May 24, 2018 • edited Loading

codebrainz commented May 24, 2018

b4n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codebrainz commented May 27, 2020 • edited Loading

codebrainz commented May 24, 2018 •

edited

Loading

evgeni commented May 24, 2018 •

edited

Loading

codebrainz commented May 27, 2020 •

edited

Loading