Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added common utils lib (2). #622

Merged
merged 1 commit into from
Oct 15, 2017
Merged

Added common utils lib (2). #622

merged 1 commit into from
Oct 15, 2017

Conversation

lpaulsen93
Copy link
Contributor

The lib is placed in sub-directory 'utils'.
So far it only includes utility functions for scanning a directory for files.

This is a fresh PR only including the changes for the common lib in folder utils. It does not include changes on other plugins. This replaces #620.

The lib is placed in sub-directory 'utils'.
So far it only includes utility functions for scanning a directory for files.
This was referenced Oct 11, 2017
@frlan frlan merged commit 6528d23 into geany:master Oct 15, 2017
@lpaulsen93 lpaulsen93 deleted the commonlib2 branch October 15, 2017 12:01
@eht16
Copy link
Member

eht16 commented Oct 15, 2017

I like the idea of this library and besides the mentioned additional use
for "relative bundle path code used on Windows and Mac", we probably can
move some of the plugin configuration config file handling in each plugin
here. There is a lot of duplicated code around.

Since we add a new dependency for other plugins here (currently just Workbench but probably
more in the future), we should emphasize this in the NEWS to help distro packagers when releasing. They probably need to add something like geany-plugins-common package and make all other plugin sub packages depend on this (as it is already on Debian).

Apropos, in order to be able to compile it on Windows I had to add:
-no-undefined to libgeanypluginutils_la_LDFLAGS.
Without, linking fails with:
can't build shared library unless -no-undefined is specified.
I'm not sure whether we should add it there or better in
build/vars-build.mk where it is already specified for AM_LDFLAGS.

@codebrainz
Copy link
Member

I also mentioned some weirdness about AM_LDFLAGS in the PR I made for the other/first PR for this, but it's mysteriously missing now. I think it was that it cannot be used by anything but plugins since it puts libtool -module -avoid-version flags in there. That is why the Makefile.am explicitly uses GP_LDFLAGS instead of implicitly inheriting the AM_LDFLAGS.

Also, I left a TODO in the .m4 template that might be nice to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants