[Github-comments] [geany/geany-plugins] Sync with latest upstream (#527)

[Colomban Wendling]

b4n approved this pull request.

:LGTM:

>  								-DGEANYPY_PYTHON_DIR="\"$(libdir)/geany/geanypy\"" \
 								-DGEANYPY_PLUGIN_DIR="\"$(libdir)/geany\"" \
 								-DG_LOG_DOMAIN=\"GeanyPy\"
-geanypy_la_CFLAGS			=	@GEANYPY_CFLAGS@ @GMODULE_CFLAGS@
+geanypy_la_CFLAGS			=	@PYGTK_CFLAGS@ @GEANY_CFLAGS@ @GEANYPY_CFLAGS@ @GMODULE_CFLAGS@

I don't think this changes matters much, but IMO it's not bad, rather good.  Yes, *pkg-config* will mostly fill the `*_CFLAGS` variable with preprocessor flags, but it might put other stuff here if it wants.  Also, it's fine putting preprocessor flags in `*_CFLAGS` AFAIK, only library order matters.

> @@ -62,7 +62,7 @@ Editor_get_property(Editor *self, const gchar *prop_name)
 		PyObject *py_doc;
 		py_doc = (PyObject *) Document_create_new_from_geany_document(
 									self->editor->document);
-		if (py_doc && py_doc != Py_None)
+		if (!py_doc || py_doc == Py_None)

Agreed, the previous one seems weird: returning `None` when `py_doc` is *not* `NULL` or `None`?  weird.

New condition seems more reasonable to me.  I wonder if `Py_RETURN_NONE` is different than `return Py_None`, but it doesn't change anything.

> @@ -0,0 +1,45 @@
+#if defined(HAVE_CONFIG_H) && !defined(GEANYPY_WINDOWS)

no license header?

-- 
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/527#pullrequestreview-22491553
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20170217/de2241cf/attachment.html>