Skip to content

Added example of using libsigc++ with Qt #35

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

Closed
wants to merge 4 commits into from

Conversation

rm5248
Copy link
Contributor

@rm5248 rm5248 commented Oct 15, 2019

No description provided.

@murraycu
Copy link
Contributor

Thanks, but what is this example showing us?

Is it showing us how to call a libsigc++ slot from a Qt signal handler (slot)? If so, is that particularly difficult to figure out, or difficult to do when also using Qt? Isn't it just like calling a function?

Or is it just an example of how you need to disable the Qt "keywords", instead using the Q_* macros?

@rm5248
Copy link
Contributor Author

rm5248 commented Oct 23, 2019

Is it showing us how to call a libsigc++ slot from a Qt signal handler (slot)? If so, is that particularly difficult to figure out, or difficult to do when also using Qt? Isn't it just like calling a function?

There's not a big difference between the Qt signals/slots and libsigc++, but they can be used in a similar manner.

Or is it just an example of how you need to disable the Qt "keywords", instead using the Q_* macros?

This is the needed part. I also wanted to show that you can use both the Qt signals/slots at the same time as libsigc++.

Because of that, it is a relatively trivial example, as otherwise it could get very complicated.

@murraycu
Copy link
Contributor

OK. Thanks. Then I'd like it to be clearer about that, please. I think it should be as minimal as possible - it should have Qt signals and slots and libsigc++ signals and slots, just to show how they can co-exist within the same class (because of the use of the Q_* keywords) but they should not interact.

And I think there should be a README.md saying what it is showing.

@rm5248
Copy link
Contributor Author

rm5248 commented Nov 3, 2019

OK, I've added a README.md and simplified the example slightly. The signals don't interact with one another much; the slot that is used for sigc++ is also a Qt slot, but I feel that it is important to mention. The one thing that the example does that is not a good practice is emitting signals inside the constructor; would it make sense to have that in a separate method for clarity?

happen correctly.

Steps to use libsigc++ with Qt:
1. In your .pro file, add `CONFIG += no_keywords`. This configures Qt to not
Copy link
Contributor

@murraycu murraycu Jan 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes that the person is using qmake, I think. But Qt doesn't require the use of qmake. I suggest renaming the example directory to "qt_with_qmake".

Also, shouldn't we link to this?
https://doc.qt.io/qt-5/signalsandslots.html#using-qt-with-3rd-party-signals-and-slots

In a CMake project, we do this like so:

add_definitions(-DQT_NO_KEYWORDS)
or, probably more "modern CMake":
target_compile_definitions(some_target PRIVATE QT_NO_KEYWORDS)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think it's worth saying that this (using the Q_* macros) is generally good advice when writing Qt code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally in agreement with that, I'll go update this in a bit. While I don't disagree that using the Q_* macros is a good idea, that's generally not what happens in most Qt code that I have seen(even Qt's own examples use emit/signals/slots and not the Q_* macros).


Steps to use libsigc++ with Qt:
1. In your .pro file, add `CONFIG += no_keywords`. This configures Qt to not
define the keywords `emit`, `signals`, and `slot`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are macros, not keywords.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're macros in the C++ land, but that is paraphrasing what the Qt documentation says in the link you put above:

It tells Qt not to define the moc keywords signals, slots, and emit [...]

@rm5248
Copy link
Contributor Author

rm5248 commented Jan 5, 2020

Updated based off of feedback.

murraycu pushed a commit that referenced this pull request Jan 8, 2020
@murraycu
Copy link
Contributor

murraycu commented Jan 8, 2020

Many thanks, and sorry for being so slow to respond.

I have squashed these into one commit and pushed them to master.

This directory is not DISTed into the tarball (by make distcheck), and we should fix that, but it's not fatal.

@murraycu murraycu closed this Jan 8, 2020
@kjellahl
Copy link
Contributor

This directory is not DISTed into the tarball (by make distcheck)

It's not distributed by make distcheck. It is distributed by ninja dist.
Meson/ninja distributes all files from the git repo, unless instructions in a
meson.build file removes some files from the meson-dist directory.

The example with Qt is not built by a meson build or an autotools build. If it's
ever added to the build systems, building it should be controlled by a configuration
option, and disabled by default, like the benchmark with Boost.

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.

3 participants