Closes #526 You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/527
-- Commit Summary --
* Sync with latest upstream
-- File Changes --
M geanypy/geany/Makefile.am (1) M geanypy/geany/__init__.py (2) A geanypy/geany/logger.py (74) M geanypy/geany/plugin.py (4) M geanypy/src/Makefile.am (7) M geanypy/src/geanypy-app.c (1) M geanypy/src/geanypy-document.c (4) M geanypy/src/geanypy-editor.c (4) A geanypy/src/geanypy-glog.c (45) M geanypy/src/geanypy-plugin.c (2) M geanypy/src/makefile.win32 (1)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/527.patch https://github.com/geany/geany-plugins/pull/527.diff
codebrainz commented on this pull request.
@@ -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)
I didn't look into this difference, I just picked the kind of condition that made the branch execute when it seemed plausible to me.
@codebrainz pushed 1 commit.
ea0a634 Fix typo on untested win32 build
kugel- commented on this pull request.
-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 think the previous code is better. *_CFLAGS derived from PKG_CONFIG_CHECK() contain include paths (and usually nothing else) so they should be added to automake's *_CPPFLAGS indeed.
One thing, the rest looks good to me.
codebrainz commented on this pull request.
-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 really mind since they all end up jammed in the same part of the command line, but it seemed logical to group the pre-processor flags and the compiler flags into their respective variables.
kugel- commented on this pull request.
-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@
*_CFLAGS derived from PKG_CONFIG_CHECK() contains mostly (usually exclusively) pre-processor flags, despite the name. And the order on the command line matters.
codebrainz commented on this pull request.
-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 guess as long as user environment `CPPFLAGS` override `CPPFLAGS` and likewise but after with `CFLAGS` it doesn't matter.
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?
codebrainz commented on this pull request.
@@ -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)
I think it's effectively like
```c #define Py_RETURN_NONE do { Py_INCREF(Py_None); return Py_None; } while (0) ```
or something similar.
codebrainz commented on this pull request.
@@ -0,0 +1,45 @@
+#if defined(HAVE_CONFIG_H) && !defined(GEANYPY_WINDOWS)
Not in each file.
Merged #527.
GeanyPy is listed in `MAINTAINERS` as “Currently diverged from upstream” — I figure that’s no longer correct?
github-comments@lists.geany.org