qtermwidget icon indicating copy to clipboard operation
qtermwidget copied to clipboard

Modernize code (Qt5 connects + init of params)

Open francescmaestre opened this issue 3 years ago • 1 comments

In this PR I've applied two main changes:

  1. Use of Qt5 connects that will cause compilation error if the signal/slot doesn't exist or has a mismatch. This prevents connection problems in runtime, quite hard to debug/spot.
  2. Move the initialization of member parameters to the header file. The reason for this is that in the case of having multiple constructors, one might forget to initialize some of the parameters, or if not forgotten, it will cause a lot of code duplication.

These should be harmless modifications since they don't change any implementation.

I've postponed some real changes like using smart pointers, modern iterators, etc. until this PR goes in and clang-format is in place. I don't think it's recommendable to modify everything in one single PR.

Last thing: I've kept one old-style connect because it would make the code use qoverload. I know that this was an issue in some distros as ArchLinux and since I'm not sure QTermWidget is ship there or not, I decided to keep it.

francescmaestre avatar Apr 10 '22 08:04 francescmaestre

Last thing: I've kept one old-style connect because it would make the code use qoverload. I know that this was an issue in some distros as ArchLinux and since I'm not sure QTermWidget is ship there or not, I decided to keep it.

We can move to that. qOverload needs Qt 5.7 and C++ 14, and LXQt now requires Qt 5.15 and C++ 17: https://github.com/lxqt/lxqt/discussions/1974, https://github.com/lxqt/lxqt-build-tools/pull/83

yan12125 avatar Jun 04 '23 16:06 yan12125