D7085: Fix issue where notifications will show as 1 pixel line
davidedmundson abandoned this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D7085 To: davidedmundson, #plasma Cc: plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10433: Add QML support for Prison
davidedmundson added a comment.
Cool in principle, few nitpicks.
INLINE COMMENTS
> barcodequickitem.cpp:37
> +BarcodeQuickItem::BarcodeQuickItem(QQuickItem *parent)
> +: QQuickPaintedItem(parent)
> +{
Is createBarcode a heavy function? It sounds like it could be.
If so I'd recommend using QQmlParserStatus so you don't generate it 4 times on
startup as each property gets set.
> barcodequickitem.cpp:113
> +
> +const auto img = m_barcode->toImage(m_barcode->minimumSize());
> +painter->drawImage(QRectF(x, y, w, h), img, img.rect());
you have the actual up-to-date item size from inside paint. Would that be
better than minimumSize?
Please also change to
img = m_barcode->toImage(size * qApp->devicePixelRatio());
img.setDevicePixelRatio(qApp->devicePixelRatio());
for high DPI support.
> barcodequickitem.cpp:131
> +m_barcode->setBackgroundColor(m_bgColor);
> +m_barcode->toImage(m_barcode->minimumSize());
> +setImplicitWidth(m_barcode->minimumSize().width());
You're doing this in the paint, do you need to do this here?
REPOSITORY
R280 Prison
REVISION DETAIL
https://phabricator.kde.org/D10433
To: vkrause, #frameworks, svuorela
Cc: davidedmundson, michaelh
D10433: Add QML support for Prison
vkrause created this revision. vkrause added reviewers: Frameworks, svuorela. Restricted Application added a project: Frameworks. vkrause requested review of this revision. REPOSITORY R280 Prison BRANCH master REVISION DETAIL https://phabricator.kde.org/D10433 AFFECTED FILES .gitignore CMakeLists.txt src/CMakeLists.txt src/quick/CMakeLists.txt src/quick/barcodequickitem.cpp src/quick/barcodequickitem.h src/quick/prisonquickplugin.cpp src/quick/qmldir tests/barcode.qml To: vkrause, #frameworks, svuorela Cc: michaelh
D10432: Set minimum size on 1D barcodes as well
vkrause created this revision. vkrause added reviewers: Frameworks, svuorela. Restricted Application added a project: Frameworks. vkrause requested review of this revision. REVISION SUMMARY This matches the behavior of all the 2D codes. REPOSITORY R280 Prison REVISION DETAIL https://phabricator.kde.org/D10432 AFFECTED FILES src/lib/code39barcode.cpp src/lib/code93barcode.cpp To: vkrause, #frameworks, svuorela Cc: michaelh
D10312: FileUndoManager: don't delete non-existing local files
elvisangelaccio added inline comments. INLINE COMMENTS > dfaure wrote in fileundomanager.cpp:404 > Where's the corresponding call to slotUnlock? > > I think you only wanted to update the state of the action, maybe better to do > that directly. You mean just an `emit undoAvailable(false);` ? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10312 To: elvisangelaccio, dfaure Cc: ngraham, #frameworks, michaelh
KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 128 - Still Unstable!
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/128/ Project: Frameworks kio kf5-qt5 SUSEQt5.10 Date of build: Sat, 10 Feb 2018 17:04:27 + Build duration: 16 min and counting JUnit Tests Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest Cobertura Report Project Coverage Summary Name PackagesFilesClassesLinesConditionalsCobertura Coverage Report64% (23/36)67% (295/442)67% (295/442)53% (31491/59492)38% (18496/48748)Coverage Breakdown by Package Name FilesClassesLinesConditionalsautotests100% (73/73)100% (73/73)94% (8549/9097)48% (5211/10759)autotests.http100% (9/9)100% (9/9)100% (586/587)59% (217/368)autotests.kcookiejar100% (1/1)100% (1/1)91% (180/198)67% (63/94)src100% (1/1)100% (1/1)100% (5/5)75% (3/4)src.core84% (101/120)84% (101/120)58% (8330/14343)50% (4871/9704)src.core.kssl100% (1/1)100% (1/1)40% (35/88)50% (3/6)src.filewidgets79% (30/38)79% (30/38)49% (3872/7838)33% (1634/4928)src.gui100% (2/2)100% (2/2)95% (104/110)77% (57/74)src.ioslaves.file100% (5/5)100% (5/5)52% (511/974)41% (412/996)src.ioslaves.file.kauth0% (0/3)0% (0/3)0% (0/104)0% (0/75)src.ioslaves.ftp0% (0/2)0% (0/2)0% (0/1365)0% (0/1515)src.ioslaves.help0% (0/5)0% (0/5)0% (0/247)0% (0/184)src.ioslaves.http89% (8/9)89% (8/9)41% (1783/4338)35% (1376/3979)src.ioslaves.http.kcookiejar33% (2/6)33% (2/6)47% (631/1333)55% (649/1174)src.ioslaves.remote100% (2/2)100% (2/2)28% (72/258)8% (19/242)src.ioslaves.remote.kdedmodule0% (0/4)0% (0/4)0% (0/14)100% (0/0)src.ioslaves.telnet0% (0/1)0% (0/1)0% (0/43)0% (0/30)src.ioslaves.trash67% (8/12)67% (8/12)5
KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.9 - Build # 108 - Still Unstable!
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.9/108/ Project: Frameworks kio kf5-qt5 FreeBSDQt5.9 Date of build: Sat, 10 Feb 2018 17:04:27 + Build duration: 9 min 15 sec and counting JUnit Tests Name: (root) Failed: 2 test(s), Passed: 55 test(s), Skipped: 0 test(s), Total: 57 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiowidgets-kdirmodeltest
KDE CI: Frameworks kio kf5-qt5 SUSEQt5.7 - Build # 126 - Still Unstable!
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.7/126/ Project: Frameworks kio kf5-qt5 SUSEQt5.7 Date of build: Sat, 10 Feb 2018 17:04:27 + Build duration: 5 min 45 sec and counting JUnit Tests Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest Cobertura Report Project Coverage Summary Name PackagesFilesClassesLinesConditionalsCobertura Coverage Report64% (23/36)67% (295/442)67% (295/442)53% (31491/59491)38% (18487/48824)Coverage Breakdown by Package Name FilesClassesLinesConditionalsautotests100% (73/73)100% (73/73)94% (8549/9097)48% (5212/10759)autotests.http100% (9/9)100% (9/9)100% (586/587)59% (217/368)autotests.kcookiejar100% (1/1)100% (1/1)91% (180/198)67% (63/94)src100% (1/1)100% (1/1)100% (5/5)75% (3/4)src.core84% (101/120)84% (101/120)58% (8330/14343)50% (4869/9708)src.core.kssl100% (1/1)100% (1/1)40% (35/88)50% (3/6)src.filewidgets79% (30/38)79% (30/38)49% (3872/7838)33% (1634/4928)src.gui100% (2/2)100% (2/2)95% (104/110)77% (57/74)src.ioslaves.file100% (5/5)100% (5/5)52% (511/974)41% (412/996)src.ioslaves.file.kauth0% (0/3)0% (0/3)0% (0/104)0% (0/75)src.ioslaves.ftp0% (0/2)0% (0/2)0% (0/1365)0% (0/1515)src.ioslaves.help0% (0/5)0% (0/5)0% (0/247)0% (0/184)src.ioslaves.http89% (8/9)89% (8/9)41% (1788/4338)35% (1373/3979)src.ioslaves.http.kcookiejar33% (2/6)33% (2/6)47% (631/1333)55% (649/1174)src.ioslaves.remote100% (2/2)100% (2/2)28% (72/258)8% (19/242)src.ioslaves.remote.kdedmodule0% (0/4)0% (0/4)0% (0/14)100% (0/0)src.ioslaves.telnet0% (0/1)0% (0/1)0% (0/43)0% (0/30)src.ioslaves.trash67% (8/12)67% (8/
D10114: Fix bug #382437 "Regression in kdialog causes wrong file extension"
ngraham closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10114 To: ijanssen, #plasma, dfaure Cc: ngraham, aacid, broulik, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10114: Fix bug #382437 "Regression in kdialog causes wrong file extension"
ijanssen added a comment. In https://phabricator.kde.org/D10114#203996, @ngraham wrote: > To commit this for you, I need an email address. Can you provide one? alave...@gmail.com REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10114 To: ijanssen, #plasma, dfaure Cc: ngraham, aacid, broulik, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10114: Fix bug #382437 "Regression in kdialog causes wrong file extension"
ngraham added a comment. Thanks. Confirmed the fix, BTW. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10114 To: ijanssen, #plasma, dfaure Cc: ngraham, aacid, broulik, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10114: Fix bug #382437 "Regression in kdialog causes wrong file extension"
ngraham added a comment. To commit this for you, I need an email address. Can you provide one? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10114 To: ijanssen, #plasma, dfaure Cc: ngraham, aacid, broulik, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D7849: Fix the tray icon scaling on HiDPI screens
pgkos added a comment. In my opinion, roundToIconSize should operate on scaled units - it is used multiple times in a few plasmoids - all calls from them to roundToIconSize assume it will operate on scaled units. The method, as it is now, is useless, because QML code has no access to KIconLoader sizes and devicePixelIconSize() method, so it cannot operate on unscaled units or convert them on the fly. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D7849 To: pgkos, #plasma Cc: ngraham, anthonyfieroni, broulik, #frameworks, davidedmundson, plasma-devel, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D7849: Fix the tray icon scaling on HiDPI screens
pgkos updated this revision to Diff 26878. pgkos added a comment. This is a simpler implementation - the diff changes roundToIconSize so it uses scaled units. REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7849?vs=19612&id=26878 REVISION DETAIL https://phabricator.kde.org/D7849 AFFECTED FILES src/declarativeimports/core/units.cpp To: pgkos, #plasma Cc: ngraham, anthonyfieroni, broulik, #frameworks, davidedmundson, plasma-devel, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10114: Fix bug #382437 "Regression in kdialog causes wrong file extension"
ijanssen added a comment. In https://phabricator.kde.org/D10114#203764, @ngraham wrote: > It doesn't merge cleanly on master. Can you re-base it? I've updated diff. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10114 To: ijanssen, #plasma, dfaure Cc: ngraham, aacid, broulik, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10114: Fix bug #382437 "Regression in kdialog causes wrong file extension"
ijanssen updated this revision to Diff 26877. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10114?vs=25981&id=26877 REVISION DETAIL https://phabricator.kde.org/D10114 AFFECTED FILES src/filewidgets/kfilewidget.cpp To: ijanssen, #plasma, dfaure Cc: ngraham, aacid, broulik, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8958: Fix unintentional breadcrumb menu item activation
aleksejshilin added a comment. In https://phabricator.kde.org/D8958#203931, @ngraham wrote: > OK cool. The patch works for me! +1. Great, thanks a lot! > Get a code review from someone else before committing. I don't have push access, so no worries. :) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8958 To: aleksejshilin, #frameworks Cc: broulik, ngraham, michaelh
D8958: Fix unintentional breadcrumb menu item activation
ngraham added a comment. OK cool. The patch works for me! +1. Get a code review from someone else before committing. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8958 To: aleksejshilin, #frameworks Cc: broulik, ngraham, michaelh
D8958: Fix unintentional breadcrumb menu item activation
aleksejshilin added inline comments. INLINE COMMENTS > ngraham wrote in kurlnavigatormenu.cpp:85 > Will this still work if the user is using the mouse in left-handed mode? Quoting Qt documentation [1]: > | Qt::LeftButton | The left button is pressed, or an event refers to the left > button. (**The left button may be the right button on left-handed mice.**) | > | Should work just fine. [1] http://doc.qt.io/qt-5/qt.html#MouseButton-enum REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8958 To: aleksejshilin, #frameworks Cc: broulik, ngraham, michaelh
D8958: Fix unintentional breadcrumb menu item activation
ngraham added inline comments.
INLINE COMMENTS
> kurlnavigatormenu.cpp:85
> +// it unless mouse was moved.
> +if (m_mouseMoved || (btn != Qt::LeftButton)) {
> +QAction *action = actionAt(event->pos());
Will this still work if the user is using the mouse in left-handed mode?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8958
To: aleksejshilin, #frameworks
Cc: broulik, ngraham, michaelh
D8958: Fix unintentional breadcrumb menu item activation
aleksejshilin updated this revision to Diff 26872. aleksejshilin added a comment. - Don't consider mouse moves which are smaller than drag distance - Don't pass the ignored mouse release event to the base class REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8958?vs=22866&id=26872 BRANCH fix_accidental_breadcrumb_menu_item_activation REVISION DETAIL https://phabricator.kde.org/D8958 AFFECTED FILES src/filewidgets/kurlnavigatormenu.cpp src/filewidgets/kurlnavigatormenu_p.h To: aleksejshilin, #frameworks Cc: broulik, ngraham, michaelh
D8958: Fix unintentional breadcrumb menu item activation
ngraham added a comment. Would you mind re-basing this on master? I can reproduce the issue and would be happy to test the patch. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8958 To: aleksejshilin, #frameworks Cc: broulik, ngraham, michaelh
D8958: Fix unintentional breadcrumb menu item activation
aleksejshilin added a comment. Hi again. :) Any news on this? Is there anything I should improve? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8958 To: aleksejshilin, #frameworks Cc: broulik, ngraham, michaelh
D10414: Add move semantics support to KIO::UDSEntry.
markg added a comment.
@dfaure i can make the test much more complete if you want.. But it's more
complex thus i left it out.
I can store the UDSEntry in a QDataStream which would be backed by a
QByteArray. That can be hashed!
Then i can do the same after moving operations and compare the hash. That way
i would know for sure the UDSEntry objects match perfectly.
But as said, that is more complex. I'd probably make a lambda within the
testMove() function. I think with a signature like this:
auto udsEntryHash = [](const KIO::UDSEntry &entry){
// some foo..
return hash; (would be the QCryptographicHash::result() output).
};
Your call :)
Or i can add that as a separate test that just tests UDSEntry copy and move
operations. That might actually be better and more generic.
Do note that this doesn't fix or catch anything at the moment. It would just
be an extra test to guarantee copying of UDSEntry objects is not broken.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10414
To: markg, dfaure
Cc: apol, #frameworks, michaelh, ngraham
D10414: Add move semantics support to KIO::UDSEntry.
markg marked 7 inline comments as done. markg added a comment. Seems my unittest skills have become a bit rusty over the years of basically not contributing patches to KDE ;) Thank you for the comments, David! REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10414 To: markg, dfaure Cc: apol, #frameworks, michaelh, ngraham
D10414: Add move semantics support to KIO::UDSEntry.
markg updated this revision to Diff 26868. markg added a comment. Update test. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10414?vs=26839&id=26868 BRANCH udsentry_reductions REVISION DETAIL https://phabricator.kde.org/D10414 AFFECTED FILES autotests/udsentrytest.cpp autotests/udsentrytest.h src/core/udsentry.cpp src/core/udsentry.h To: markg, dfaure Cc: apol, #frameworks, michaelh, ngraham
D10405: Don't proceed in runCommandInternal if the executable doesn't exit
ngraham accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10405 To: hein, dfaure, davidedmundson, mart, ngraham Cc: #frameworks, michaelh, ngraham
D10414: Add move semantics support to KIO::UDSEntry.
dfaure added inline comments. INLINE COMMENTS > udsentrytest.cpp:227 > +/** > + * Test to verify that move semantics work. This is only useful when ran > through callgrind. > + */ ... or gdb, or perf, or with debug output in the cpp file, or > udsentrytest.cpp:234 > +QVERIFY(file.open()); > +const QByteArray filePath = file.fileName().toLocal8Bit(); > +const QString fileName = QUrl(file.fileName()).fileName(); // > QTemporaryFile::fileName returns the full path. QFile::encodeName() is the proper way > udsentrytest.cpp:235 > +const QByteArray filePath = file.fileName().toLocal8Bit(); > +const QString fileName = QUrl(file.fileName()).fileName(); // > QTemporaryFile::fileName returns the full path. > +QVERIFY(!fileName.isEmpty()); That's rather overkill (and a wrong use of the QUrl API). You want to use QFileInfo for this. > udsentrytest.cpp:240 > +QT_STATBUF statBuf; > +QVERIFY(QT_LSTAT(filePath.constData(), &statBuf) == 0); > +KIO::UDSEntry entry(statBuf, fileName); Whenever you write QVERIFY(a==b), it should be QCOMPARE(a, b) > udsentrytest.cpp:256 > + > +// And veryfy that this works. > +QCOMPARE(fileName, movedEntry.stringValue(KIO::UDSEntry::UDS_NAME)); typo: verify > udsentrytest.cpp:268 > + > +// And veryfy that this works. > +QCOMPARE(fileName, movedEntry.stringValue(KIO::UDSEntry::UDS_NAME)); same typo > udsentrytest.cpp:269 > +// And veryfy that this works. > +QCOMPARE(fileName, movedEntry.stringValue(KIO::UDSEntry::UDS_NAME)); > +} swap it around: what you test on the left, what you expect on the right. This makes error messages more readable. (same above, of course) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10414 To: markg, dfaure Cc: apol, #frameworks, michaelh, ngraham
D10365: New icon for Elisa music player
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks for your work. I am hesitant about the choice of an audio tape. I fear people under 30 will not recognize it. I would like to propose trying the tape with the current color and we can revisit this decision in 6 months. I agree on the text removal and your choice of color is good for me. REPOSITORY R266 Breeze Icons BRANCH master REVISION DETAIL https://phabricator.kde.org/D10365 To: paullesur, #breeze, #vdg, #elisa, andreaska, mgallien Cc: mgallien, januz, astippich, andreask, andreaska, ltoscano, ngraham, #frameworks, paullesur, michaelh, ognarb, kmf, progwolff
