ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> autostart.cpp:167
> {
> - KOpenWithDialog owdlg( this );
> - if (owdlg.exec() != QDialog::Accepted)
> - return;
> + KOpenWithDialog* owdlg = new KOpenWithDialog(this);
> + connect(owdlg, &QDialog::finished, this, [this, owdlg] (int result) {
Space before * not after, or just use auto
> autostart.cpp:186
> {
> - AddScriptDialog * addDialog = new AddScriptDialog(this);
> - int result = addDialog->exec();
> - if (result == QDialog::Accepted) {
> - if (addDialog->symLink())
> - KIO::link(addDialog->importUrl(),
> QUrl::fromLocalFile(m_paths[0]));
> - else
> - KIO::copy(addDialog->importUrl(),
> QUrl::fromLocalFile(m_paths[0]));
> -
> - ScriptStartItem * item = new ScriptStartItem( m_paths[0] +
> addDialog->importUrl().fileName(), m_scriptItem,this );
> - addItem( item, addDialog->importUrl().fileName(),
> addDialog->importUrl().fileName(),ScriptStartItem::START );
> - }
> - delete addDialog;
> + AddScriptDialog* addDialog = new AddScriptDialog(this);
> + connect(addDialog, &QDialog::finished, this, [this, addDialog] (int
> result) {
ditto
> autostart.cpp:214
> {
> + Q_UNUSED(parent)
> + Q_UNUSED(last)
Actually I think I'd Q_ASSERT(!parent.isValid())
> autostart.cpp:215
> + Q_UNUSED(parent)
> + Q_UNUSED(last)
>
This Q_UNUSED is now used ;-)
> autostart.cpp:276
> +
> + KPropertiesDialog* dlg = new KPropertiesDialog( kfi, this );
> + connect(dlg, &QDialog::finished, this, [this, index, fileName,
> desktopItem, dlg] (int result) {
Space before * not after, or use auto
Also drop the spaces between the parentheses
> autostart.cpp:296
> {
> - if ( widget->listCMD->currentItem() == nullptr )
> + if ( widget->listCMD->currentItem() == nullptr ) {
> return;
Since you touched the line, please drop the spaces between the parentheses
> autostart.cpp:304
> {
> - if ( widget->listCMD->currentItem() == nullptr )
> + if ( widget->listCMD->currentItem() == nullptr ) {
> return;
Since you touched the line, please drop the spaces between the parentheses
> autostart.cpp:321
> +
> +QModelIndex Autostart::indexFromWidget(QTreeWidgetItem* widget) const {
> + int index = m_programItem->indexOfChild(widget);
Since you touched the line please fix the * and { positions
> ervin wrote in autostart.h:64
> Still needs being addressed
Still needs being addressed. :-)
> autostartmodel.cpp:130
> +
> +AutostartModel::AutostartModel(QWidget* parent)
> + : QAbstractListModel(parent)
Space before * not after
> ervin wrote in autostartmodel.cpp:177
> Makes me realize: are we sure it will always be the first entry? I'm not sure
> we can guarantee that over time.
Still needs being addressed.
> autostartmodel.h:49
> +public:
> + explicit AutostartModel(QWidget* parent = nullptr);
> +
Since you touched the line, please drop the spaces between the parentheses
> autostartmodel.h:81
> + QVector<AutostartEntry> m_entries;
> + QWidget* m_window;
> +};
Space before * not after
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D26934
To: meven, mlaurent, ervin, #plasma, broulik, bport, crossi
Cc: alex, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh,
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
