D21354: Port to new connect syntax

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R432:95df6fa38a45: Port to new connect syntax (authored by ngraham). REPOSITORY R432 File Sharing (Samba) integration CH

D21354: Port to new connect syntax

ngraham added a comment. In D21354#469531 , @bruns wrote: > LGTM > > currently, GCC thows some "0 for nullptr" and "missing override" warnings, are you going to address these (new SR)? > > Also, Qt recommends deriving from Q**Styled**Item

D21354: Port to new connect syntax

bruns added a comment. LGTM currently, GCC thows some "0 for nullptr" and "missing override" warnings, are you going to address these (new SR)? Also, Qt recommends deriving from Q**Styled**ItemDelegate REPOSITORY R432 File Sharing (Samba) integration REVISION DETAIL https://pha

D21354: Port to new connect syntax

ngraham updated this revision to Diff 58607. ngraham added a comment. Rebase REPOSITORY R432 File Sharing (Samba) integration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21354?vs=58605&id=58607 BRANCH port-to-new-connect-symtax (branched from master) REVISION DETAIL https

D21354: Port to new connect syntax

ngraham updated this revision to Diff 58605. ngraham marked an inline comment as done. ngraham added a comment. Don't double-connect REPOSITORY R432 File Sharing (Samba) integration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21354?vs=58568&id=58605 BRANCH port-to-new-connec

D21354: Port to new connect syntax

bruns added inline comments. INLINE COMMENTS > sambausershareplugin.cpp:130 > +connect(propertiesUi.sambaNameEdit, &QLineEdit::textChanged, > +this, &SambaUserSharePlugin::checkShareName); > +connect(propertiesUi.sambaAllowGuestChk, &QCheckBox::toggled, I meant - do `setDirty

D21354: Port to new connect syntax

ngraham marked 10 inline comments as done. ngraham added a comment. Yep, totally works without that connect(). REPOSITORY R432 File Sharing (Samba) integration REVISION DETAIL https://phabricator.kde.org/D21354 To: ngraham, #frameworks, apol Cc: bruns, aacid

D21354: Port to new connect syntax

ngraham updated this revision to Diff 58568. ngraham added a comment. Address review comments REPOSITORY R432 File Sharing (Samba) integration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21354?vs=58513&id=58568 BRANCH port-to-new-connect-symtax (branched from master) REVISI

D21354: Port to new connect syntax

bruns added inline comments. INLINE COMMENTS > ngraham wrote in delegate.cpp:46 > Nope, same issue I'm afraid. GCC 8.3.1 gives me a helpful error message: ...samba/filepropertiesplugin/delegate.cpp: In lambda function: ...samba/filepropertiesplugin/delegate.cpp:48:58: error: passing ‘const

D21354: Port to new connect syntax

ngraham added inline comments. INLINE COMMENTS > aacid wrote in delegate.cpp:46 > without having tried it, you probably still need the QOverload to say which > activated version you want, so > > connect(comboBox, QOverload::of(&QComboBox::activated), this, [this, > comboBox] { emit commitData(

D21354: Port to new connect syntax

aacid added inline comments. INLINE COMMENTS > ngraham wrote in delegate.cpp:46 > I'd love to, and I tried, but this stuff is pretty new to me and I kept > getting `error: cannot define member function` What's wrong with this? > > connect(comboBox, &QComboBox::activated, > [comboBox

D21354: Port to new connect syntax

bruns added inline comments. INLINE COMMENTS > ngraham wrote in delegate.cpp:46 > I'd love to, and I tried, but this stuff is pretty new to me and I kept > getting `error: cannot define member function` What's wrong with this? > > connect(comboBox, &QComboBox::activated, > [comboBox

D21354: Port to new connect syntax

ngraham added inline comments. INLINE COMMENTS > bruns wrote in delegate.cpp:46 > use a lambda here: > > - avoids the single-use `emitCommitData` wrapper > - avoids the need for `qobject_cast(sender())`, which is the > combobox (just capture it). I'd love to, and I tried, but this stuff is pre

D21354: Port to new connect syntax

bruns added inline comments. INLINE COMMENTS > sambausershareplugin.cpp:126 > +connect(propertiesUi.sambaChk, &QCheckBox::toggled, > +this, &SambaUserSharePlugin::changed); > +connect(propertiesUi.sambaNameEdit, &QLineEdit::textChanged, I think instead of emiting `KFileProper

D21354: Port to new connect syntax

bruns added inline comments. INLINE COMMENTS > delegate.cpp:46 > +connect(comboBox, QOverload::of(&QComboBox::activated), > +this, &UserPermissionDelegate::emitCommitData); > use a lambda here: - avoids the single-use `emitCommitData` wrapper - avoids the need for `qobject_cas

D21354: Port to new connect syntax

aacid added inline comments. INLINE COMMENTS > delegate.h:41 > +private: > +void emitCommitData(int index); > }; why add the int if you're not going to use it? REPOSITORY R432 File Sharing (Samba) integration REVISION DETAIL https://phabricator.kde.org/D21354 To: ngraham, #frameworks

D21354: Port to new connect syntax

ngraham updated this revision to Diff 58513. ngraham added a comment. Remove extra space REPOSITORY R432 File Sharing (Samba) integration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21354?vs=58512&id=58513 BRANCH port-to-new-connect-symtax (branched from master) REVISION DE

D21354: Port to new connect syntax

ngraham created this revision. ngraham added reviewers: Frameworks, apol. ngraham requested review of this revision. TEST PLAN - It compiles - Tested all functionality; everything works (well... at least everything that was working before is still working now :) ) REPOSITORY R432 File Shar