Skip to content

Upgrade a few classes to newer Qt connect style#1846

Merged
vadi2 merged 2 commits intoMudlet:developmentfrom
vadi2:upgrade-to-qt5-connect
Jul 26, 2018
Merged

Upgrade a few classes to newer Qt connect style#1846
vadi2 merged 2 commits intoMudlet:developmentfrom
vadi2:upgrade-to-qt5-connect

Conversation

@vadi2
Copy link
Copy Markdown
Member

@vadi2 vadi2 commented Jul 25, 2018

Brief overview of PR changes/additions

Upgrade a few classes to newer connect style.

Motivation for adding to Mudlet

More modern connect style which is safer and less frustrating to use.

Other info (issues closed, discussion etc)

I avoided changing classes that #1840 also edits to reduce merge conflicts.

@vadi2 vadi2 requested a review from a team as a code owner July 25, 2018 05:29
@vadi2 vadi2 requested a review from a team July 25, 2018 05:29
@vadi2 vadi2 changed the title Upgrade a few classes to newer connect style Upgrade a few classes to newer Qt connect style Jul 25, 2018
Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

I am beginning to get my head around these! 😀

Please consider merging in vadi2#12 in which I have converted most of the remaining Mudlet based ones - including the ones that need extra stuff to get around signal overloading (particularly relevant to valueChanged/currentIndexChange on QSpinBox/QComboBox type controls) - and some that you seem to have missed {it seems that it takes the IDE a short time after updating one instance of connect/disconnect before it re-enables the optimisation for another one - with the result that it can give the impression that the second is not so optimise-able...}!

Also - can you revert the changes to dlgColorTrigger as those clash with PR #1840 and I have already made these changes to the (different) set of controls in that version of that class?

Include those that need a qOverload<...>(...) wrapper to disambigute
overloaded signal sources.

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
@vadi2 vadi2 requested a review from a team July 26, 2018 04:06
@vadi2
Copy link
Copy Markdown
Member Author

vadi2 commented Jul 26, 2018

Merged, thanks! I haven't touched dlgColorTrigger though as mentioned.

Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

Ah, I got the location of the clashes wrong - though it does impact the dlgColorTrigger class - the three connects in the dlgTriggerEditor constructor changed make merging harder:

     for (int i = 0; i < 50; i++) {
         auto pItem = new dlgTriggerPatternEdit(HpatternList);
         QStringList _patternList;
         _patternList << "substring"
                      << "perl regex"
                      << "begin of line substring"
                      << "exact match"
                      << "Lua function"
                      << "line spacer"
                      << "color trigger"
                      << "prompt";
         QComboBox* pBox = pItem->comboBox_patternType;
         pBox->addItems(_patternList);
         pBox->setItemData(0, QVariant(i));
-        connect(pBox, SIGNAL(currentIndexChanged(int)), this, SLOT(slot_set_pattern_type_color(int)));
-        connect(pItem->pushButton_fgColor, SIGNAL(pressed()), this, SLOT(slot_color_trigger_fg()));
-        connect(pItem->pushButton_bgColor, SIGNAL(pressed()), this, SLOT(slot_color_trigger_bg()));
+        connect(pBox, qOverload<int>(&QComboBox::currentIndexChanged), this, &dlgTriggerEditor::slot_set_pattern_type_color);
+        connect(pItem->pushButton_fgColor, &QAbstractButton::pressed, this, &dlgTriggerEditor::slot_color_trigger_fg);
+        connect(pItem->pushButton_bgColor, &QAbstractButton::pressed, this, &dlgTriggerEditor::slot_color_trigger_bg);
         HpatternList->layout()->addWidget(pItem);
         mTriggerPatternEdit.push_back(pItem);
         pItem->mRow = i;
         pItem->pushButton_fgColor->hide();
         pItem->pushButton_bgColor->hide();
         pItem->pushButton_prompt->hide();
         pItem->label_patternNumber->setText(QString::number(i+1));
         pItem->label_patternNumber->show();
     }

OTOH I may have shot myself in the foot there by changing those lines in both PRs 🤦‍♂️ - never mind - I'll resolve that in the other one after this has gone in...

Am I allowed to be the Approver given that I have also lobbed a load of the changes in - it seems so!

@vadi2 vadi2 merged commit 6572dc9 into Mudlet:development Jul 26, 2018
@vadi2 vadi2 deleted the upgrade-to-qt5-connect branch July 26, 2018 11:30
@vadi2
Copy link
Copy Markdown
Member Author

vadi2 commented Jul 26, 2018

Merged so we can continue working while windows build is acting up.

SlySven added a commit to SlySven/Mudlet that referenced this pull request Aug 2, 2018
# By Vadim Peretokin (81) and others
* development: (162 commits)
  BugFix: set Server Encoding correctly on auto-loading profiles
  Install xz-utils to guarantee xz is available
  Create and upload source tarballs on release
  BugFix: use Alternative OpenSSLBinary for Windows (Mudlet#1850)
  Fix auto-save kicking in and blocking save profile as
  Upgrade a few classes to newer Qt connect style (Mudlet#1846)
  Fix package exporter to work with async save (Mudlet#1832)
  Pulled out actions into a mudlet member class variables (Mudlet#1839)
  Create module zip if it's not already created when syncing (Mudlet#1842)
  i18n-ise GUI label creation (Mudlet#1838)
  Align "no map"message in centre propely (Mudlet#1837)
  New Crowdin translations (Mudlet#1756)
  Delete old version checks (Mudlet#1833)
  Re-set dev.
  3.11.1 bugfix release.
  Fix iterator to actually iterate
  BugFix: fix faulty log options for new profiles or old profile save files
  Big5 support (Mudlet#1808)
  BugFix: include a fail-back icon for the auto-saved profile in Con. Dialog
  Back to development we go!
  ...

Conflicts resolved in:
* src/TTextEdit.cpp
* src/TTextEdit.h

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
SlySven added a commit to SlySven/Mudlet that referenced this pull request Jun 22, 2020
# By Vadim Peretokin (24) and others
# Via keneanung
* development: (52 commits)
  Install xz-utils to guarantee xz is available
  Create and upload source tarballs on release
  BugFix: use Alternative OpenSSLBinary for Windows (Mudlet#1850)
  Fix auto-save kicking in and blocking save profile as
  Upgrade a few classes to newer Qt connect style (Mudlet#1846)
  Fix package exporter to work with async save (Mudlet#1832)
  Pulled out actions into a mudlet member class variables (Mudlet#1839)
  Create module zip if it's not already created when syncing (Mudlet#1842)
  i18n-ise GUI label creation (Mudlet#1838)
  Align "no map"message in centre propely (Mudlet#1837)
  New Crowdin translations (Mudlet#1756)
  Delete old version checks (Mudlet#1833)
  Re-set dev.
  3.11.1 bugfix release.
  Fix iterator to actually iterate
  BugFix: fix faulty log options for new profiles or old profile save files
  Big5 support (Mudlet#1808)
  BugFix: include a fail-back icon for the auto-saved profile in Con. Dialog
  Back to development we go!
  3.11.0 release
  ...

Conflicts resolved in:
* src/dlgProfilePreferences.cpp
* src/mudlet.qrc

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
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.

2 participants