Markdown-specific changes from #677. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/746
-- Commit Summary --
* Markdown: add support for webkit2gtk
-- File Changes --
M build/markdown.m4 (4) M markdown/src/Makefile.am (4) M markdown/src/viewer.c (29) M markdown/src/viewer.h (6)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/746.patch https://github.com/geany/geany-plugins/pull/746.diff
codebrainz commented on this pull request.
@@ -49,13 +49,13 @@ AC_DEFUN([GP_CHECK_MARKDOWN],
GTK_VERSION=2.16 WEBKIT_VERSION=1.1.13
- GP_CHECK_GTK3([webkit_package=webkitgtk-3.0], + GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0],
@hyperair I wonder if we can cascade from `webkit-1.0` to `webkit-3.0` to `webkitgtk-4.0` to try and support all versions?
codebrainz commented on this pull request.
@@ -49,13 +49,13 @@ AC_DEFUN([GP_CHECK_MARKDOWN],
GTK_VERSION=2.16 WEBKIT_VERSION=1.1.13
- GP_CHECK_GTK3([webkit_package=webkitgtk-3.0], + GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0],
Not tested, but I mean like this:
```sh GP_CHECK_GTK3([webkit_package=webkit-3.0], [ GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0], [ webkit_package=webkit-1.0 ] ]) ... AM_CONDITIONAL([MARKDOWN_WEBKIT2], [test "x$webkit_package" = "xwebkit2gtk-4.0"]) ... ```
@b4n @sardemff7 @dmaphy @evgeni @hyperair care to comment?
If you don’t go with fallback checks, better drop the `#else`s. Other comments inside.
sardemff7 commented on this pull request.
If you don’t go with fallback checks, better drop the `#else`s. Other comments inside.
@@ -35,4 +35,8 @@ markdown_la_CFLAGS += $(LIBMARKDOWN_CFLAGS)
markdown_la_LIBADD += $(LIBMARKDOWN_LIBS) endif
+if MARKDOWN_WEBKIT2 +markdown_la_CFLAGS += -DMARKDOWN_WEBKIT2
Why not an `AC_DEFINE` in `markdown.m4` instead?
@@ -49,13 +49,13 @@ AC_DEFUN([GP_CHECK_MARKDOWN],
GTK_VERSION=2.16 WEBKIT_VERSION=1.1.13
- GP_CHECK_GTK3([webkit_package=webkitgtk-3.0], + GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0],
It should work, but you better not add fallback code for unsupported versions, distributions with older packages will use older plugin too. (And people really must stop thinking everything will always work for them without patching on older systems, do not but the maintenance burden on you.)
codebrainz commented on this pull request.
@@ -49,13 +49,13 @@ AC_DEFUN([GP_CHECK_MARKDOWN],
GTK_VERSION=2.16 WEBKIT_VERSION=1.1.13
- GP_CHECK_GTK3([webkit_package=webkitgtk-3.0], + GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0],
I think the versions listed are "supported" in that they're known to build and work for the plugin, but not that they are "supported" upstream, obviously. IMO, if it's a simple matter of a few ugly `#ifdef` it's worthwhile even if just to allow people on slightly older distros to compile the latest plugin release tarball.
codebrainz commented on this pull request.
@@ -35,4 +35,8 @@ markdown_la_CFLAGS += $(LIBMARKDOWN_CFLAGS)
markdown_la_LIBADD += $(LIBMARKDOWN_LIBS) endif
+if MARKDOWN_WEBKIT2 +markdown_la_CFLAGS += -DMARKDOWN_WEBKIT2
Yeah, it does seem better.
TODO: rebase ontop of #747.
hyperair commented on this pull request.
@@ -35,4 +35,8 @@ markdown_la_CFLAGS += $(LIBMARKDOWN_CFLAGS)
markdown_la_LIBADD += $(LIBMARKDOWN_LIBS) endif
+if MARKDOWN_WEBKIT2 +markdown_la_CFLAGS += -DMARKDOWN_WEBKIT2
`AC_DEFINE` affects everything, so plugins that don't care about this variable will get it too. I originally wanted to just have a `WEBKIT2` macro defined only for `markdown` based on the `MARKDOWN_WEBKIT2` conditional, but @codebrainz changed that.
codebrainz commented on this pull request.
@@ -35,4 +35,8 @@ markdown_la_CFLAGS += $(LIBMARKDOWN_CFLAGS)
markdown_la_LIBADD += $(LIBMARKDOWN_LIBS) endif
+if MARKDOWN_WEBKIT2 +markdown_la_CFLAGS += -DMARKDOWN_WEBKIT2
It won't clash because I added the namespace prefix, so it doesn't seem like an issue.
hyperair commented on this pull request.
@@ -49,13 +49,13 @@ AC_DEFUN([GP_CHECK_MARKDOWN],
GTK_VERSION=2.16 WEBKIT_VERSION=1.1.13
- GP_CHECK_GTK3([webkit_package=webkitgtk-3.0], + GP_CHECK_GTK3([webkit_package=webkit2gtk-4.0],
Might be useful -- on Ubuntu's side, there's still one remaining release (trusty) that has webkitgtk-3.0 but not webkit2gtk-4.0.
@codebrainz Would you mind on doing thementioned rebase and I'd merge it
Merged #746.
github-comments@lists.geany.org