Scope: fix Automake conditional when building for Windows

[b4n]

It is not valid to place AM_CONDITIONALs inside a shell condition, because Automake has to known its value in all cases. Automake is smart enough to detect it if used in a AutoShell conditional (like AS_IF/AS_CASE/...), but can't work any magic inside a pure shell conditional (if/case/...). So, move the AM_CONDITIONAL outside the conditional, as the value is already conditional anyway.

This PR also cleans up the conditional a little bit by giving it a plugin-specific name, and removing irrelevant test fro the condition (whether or not the plugin is enabled has little to no relevance here, and the plugin won't be built anyway).

[b4n]

See the build failure there: https://nightly.geany.org/win32/build_win32_plugins_stderr.log

[lpaulsen93]

LGBI. The build error on Linux and GTK3 seems to only appear on Travis CI environment.

[elextr]

The build error on Linux and GTK3 seems to only appear on Travis CI environment.

The CI needs to be set up to get the GTK3 VTE library I think.

[lpaulsen93]

The CI needs to be set up to get the GTK3 VTE library I think.

Yes. I assume the entry "libvte-dev" in the config needs to be replaced with "libvte-2.91-dev". But I am not sure what that will do to other plugins.

[elextr]

Strangely Geany builds with VTE for both GTK2 and GTK3, but its not mentioned in its .travis.yml so how does it get it?

[b4n]

@elextr Geany doesn't depend on libvte at all, it dynamically loads it and has it's own copy of the prototypes and constants and such -- see vte.c.

[elextr]

Geany doesn't depend on libvte at all

More worried about it not depending on <vte/vte.h>

Instead it silently depends on VTE devs not changing anything in the structs or consts, well, G* stuff has a great record for that, though to be fair so far we seem to have gotten away with it.

[elextr]

And HAVE_VTE actually means WANT_VTE_AND_AM_GUESSING_THE_API 😁

[b4n]

LGBI. The build error on Linux and GTK3 seems to only appear on Travis CI environment.

Nah it's my bad, dddd4bc removes the check for whether Scope is enabled because I thought it was redundant but didn't realize this conditional was in utilslib…

…which IMO is a very bad thing. BTW, as is I doubt the vtecompat thing in utilslib makes much sense, because it seems highly specific to Scope's use of it, but well.

Anyway, the test should be in the corresponding module, so as it's in utilslib it should be in the utils checks.
I did that in the latest version of this PR.

[b4n]

OK, as for Travis, it doesn't have libvte-2.91, but it does have libvte-2.90, which should likely be usable, maybe with a little bit of care (IIRC, 2.91 switched to using GdkRGBA instead of GdkColor).

[b4n]

OK, so I have commits to check for VTE-2.90 as well, but it will need actual code support to be useful, so I took it out from this PR. If it's wanted, I can push it somewhere again.

[codebrainz]

…which IMO is a very bad thing. BTW, as is I doubt the vtecompat thing in utilslib makes much sense, because it seems highly specific to Scope's use of it, but well.

Presumably the idea was that since several plugins use the VTE library, eventually in the future the other plugins' code/checks/compat could be factored out into the common utils library.

[lpaulsen93]

Presumably the idea was that since several plugins use the VTE library, eventually in the future the other plugins' code/checks/compat could be factored out into the common utils library.

Correct. I have put some Gtk and Vte compatibility stuff in the common utils lib as there was no attempt for shared compatibility code yet. Also, regarding Vte I wanted to prevent that the utils lib always depends on Vte even if no plugin is enabled which uses it. That's why there is this "cross-reference" in the makefiles. I didn't know a better way to do it. If there is a cleaner way for archiving this then I am open for suggestions.

Vte compatibility code might not be used often but there are definitely some plugins which still need to be ported to Gtk3. So I have made a first attempt to introduce some code in PR #750. There was a discussion about it some time ago but not much feedback IIRC. The bottom line is: this is a first attempt - suggestions/feedback/PRs are welcome. It won't be to much work adjusting the scope plugin to changes of the utils lib I think.

[b4n]

Also, regarding Vte I wanted to prevent that the utils lib always depends on Vte even if no plugin is enabled which uses it. That's why there is this "cross-reference" in the makefiles. I didn't know a better way to do it. If there is a cleaner way for archiving this then I am open for suggestions.

Look at 347a0c1 :)

Vte compatibility code might not be used often but there are definitely some plugins which still need to be ported to Gtk3. […]

I'm not saying that it's a bad thing to have VTE compatibility code in that utils lib, but that in the current state it doesn't seem super friendly for other users to me. But indeed it can be improved and made to fit in the future as consumers grow.

[elextr]

But indeed it can be improved and made to fit in the future as consumers grow.

Its the old thing with a library, do you build it and hope users come, or do you wait for users to demand then build it?

[lpaulsen93]

Look at 347a0c1 :)

Saw it later, sorry :-) One question: could the Travis CI build be extended to also build for Windows to recognize Windows build related errors sooner?

[eht16]

could the Travis CI build be extended to also build for Windows

Should be possible as we cross-compile also on Linux in the nightly builds setup. As usual, it just needs someone to port it to Travis. If someone wants to start, I can provide the necessary GTK stack (i.e. the files we use for the nightly builds).

[eht16]

Just tested the changes on native Windows:
configure runs fine but building Scope fails with scope.c:28:10: fatal error: vte/vte.h: No such file or directory :(.
After removing the "vte.h" include in scope.c it builds fine on Windows and the include seems unnecessary to me after reading f0dbaba or am I missing something?

[elextr]

After removing the "vte.h" include in scope.c it builds fine on Windows and the include seems unnecessary to me after reading f0dbaba or am I missing something?

Looks unused to me too.

Note that the include of <vte/vte.h> in conterm.c is guarded by OS_UNIX so if its needed in scope.c then it (and whatever uses it) should be guarded too.

[lpaulsen93]

After removing the "vte.h" include in scope.c it builds fine on Windows and the include seems unnecessary to me after reading f0dbaba or am I missing something?

Yes, my fault. It's unused now. Maybe I needed it in some intermittent version of the code and forgot to remove it later. All calls to vte should be in conterm.c. There also seems a unnecessary include in prefs.c. Will re-check in the evening.

[lpaulsen93]

I removed the #include <vte/vte.h> lines from scope.c and prefs.c and it still builds fine for GTK2 and 3 on Linux.

[eht16]

@LarsGit223 did you oush your changes regarding the vte.h include?
Besides this, is here anything missing to be merged?

[lpaulsen93]

@eht16: no, I didn't. I already have a fork from the geany main repository and don't know how to fork a second one from b4n's repository.

Anyway, once this is merged I can update and deliver a PR with the changes.

[codebrainz]

I already have a fork from the geany main repository and don't know how to fork a second one from b4n's repository.

https://git-scm.com/book/en/v2/Git-Basics-Working-with-Remotes

[lpaulsen93]

@codebrainz: sorry, but how does that help me? I used git remote before e.g. to sync my fork of a repository with the original repo. I know I could locally clone b4n's repo on the CLI, but how do I get my changes back? I can't push of course because I don't have the permission. And a PR is a pure github function. So what would be the workflow? It is not clear to me from the git documentation.

[elextr]

@LarsGit223 can't you fork b4n/geany-plugins and make a pull request on it? Not that that is visible on this repository, thats one of the failings of github, its not transitive :)

[lpaulsen93]

@elextr: no, because I already have a fork from geany/geany-plugins. github just says "You've already forked geany-plugins" if I click on fork in b4n/geany-plugins repo.

[elextr]

Well theres ya problem :) Stupid github.

Guess all you can do is pull b4n's changes into your fork using Git and then publish a PR to G-P with those and your additional changes.

[b4n]

@LarsGit223 other solution is just show me the commit and I add it :)

But @elextr and @codebrainz are right, you can easily just pull my changes into your Geany clone repo: you add my remote, and you fetch suff from it.

[lpaulsen93]

@b4n, @elextr, @codebrainz: ok, I took the simplest way and just created a PR for the changes of the includes as the change is not depending on b4n's changes (see #793).

@b4n, @codebrainz: sorry. I tried fetching b4n's repo but it seems to fetch everything out of it. Merging into one of my branches was not a problem. But I was afraid to push back in my github repo cause I feared that it would push all new branches into it. So I took the easy/save way.