search

tibirna / qgit

Official git repository for QGit.
Other
174 stars 68 forks source link

Check the result returned by the function QObject::connect() #1

Closed hkarel closed 6 years ago

hkarel commented 7 years ago

The chk_connect macro is used to check the result returned by the function QObject::connect() in debug mode, it looks like on assert() function. However, in the release mode, unlike the assert() function - test expression is not removed. To use the macro in the code need include "assert.h"

tibirna commented 7 years ago

Hello hkarel

Thanks for what appears to be huge manual work.

I wonder what is the advantage of adding Q_ASSERT on all connects vs. using the QT_FATAL_WARNINGS=1 environment variable when compiling in debug mode?

hkarel commented 7 years ago

Please, do not replace -pedantic. Newer compilers still understand it. Older compilers don't understand -Wpendantic. No need to introduces unnecessary limitations to usable compilers

Done. But what do I do next? At me are ready the commits with support for C ++ 11 standard. How it is long planned to adhere to old compilers? What it's compilers are?

hkarel commented 7 years ago

Thanks for what appears to be huge manual work.

In fact, it isn't a lot :) (AutoCorrect). I really wanted to format the code after AutoCorrect, but I was afraid, because you and I have different sizes of padding.

I wonder what is the advantage of adding Q_ASSERT on all connects vs. using the QT_FATAL_WARNINGS=1 environment variable when compiling in debug mode?

M-m-m, I did not know about such a variable. I decided to try it. The program is stopped at line textEdit Diff-> set ReadOnly (true); After which execution was interrupted. I guess I would not like to dwell on all the warnings in a row. At the same time chk_connect macro pursues a specific task: check the correct connection SIGNAL/SLOT. And nothing more. I'm not saying that the solution is perfect if you have a better varaint - offer. But what exactly is it that this macro more than once saved me from stupid missteps.

tibirna commented 7 years ago

I didn't look yet at your C++11 modifications. Could you please explain in a few words what are the goals and benefits of these modifications?

I personally wholeheartedly agree with "upgrading" to C++11 (at least). It makes sense, in 2016. There is one criteria when deciding to put it in the master branch or not: what qgit's "clients" think.

For qgit, the immediate clients are Linux distributions that want to compile and distribute it. If there are Linux distros that still ship older C++ compiler, without support for C++11, then we can't easily impose a default C++11 codebase. The distros won't have their hand forced and they'd just drop QGit.

So, we have to find out if any distros are still shipping old C++ compilers.

That said, it should still be possible to offer a C++11-compatible QGit branch for a while. This will require a little bit of work to keep the "classical" and the C++11 branches in sync, but it might be useful.

hkarel commented 7 years ago

I didn't look yet at your C++11 modifications. Could you please explain in a few words what are the goals and benefits of these modifications?

I was planning to make these changes as follows "pull request". Now it is in my repository: https://github.com/hkarel/qgit/tree/ru-lang For example see declaration of class Rev. In fact, with c++11 constructions isn't a lot, and is possible without them. You ask: What is the meaning, what the purpose of the use of C++11? A what's the point of using Qt for the design of graphical user interface (rhetorical question)? It's just convenient. This gives new opportunities for writing code. Improves its quality. Unless it isn't enough?

If there are Linux distros that still ship older C++ compiler, without support for C++11...

I think this applies to distributions to 2012 inclusive. Modern Linux distributions, without support for C++11, I do not know. In any case I to myself didn't install such the distributive :) By the way, I have installed not new the distro Kubuntu 14.04 LTS. The support of c++11 - is fine.

That said, it should still be possible to offer a C++11-compatible QGit branch for a while. This will require a little bit of work to keep the "classical" and the C++11 branches in sync, but it might be useful.

I think that this is possible, moreover, that the difference is not much. But first is necessary to resolve this "pull request".