D18804: Workaround for the bug 393630

anthonyfieroni added a comment. Did you understand why it happen? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D18804 To: trmdi, broulik, davidedmundson, fvogt, anthonyfieroni, ngraham, #plasma Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18798: [KIO/drag and drop] Fix file and folder drag and drop popup menu transparency

anthonyfieroni added a reviewer: davidedmundson. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D18798 To: anemeth, #frameworks, kde-frameworks-devel, ngraham, davidedmundson Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D18680: Fix ENABLE_CLAZY option with clazy >= 1.5

anthonyfieroni added inline comments. INLINE COMMENTS > KDECMakeSettings.cmake:296 > if(ENABLE_CLAZY) > -set(CMAKE_CXX_COMPILE_OBJECT "${CMAKE_CXX_COMPILE_OBJECT} -Xclang > -load -Xclang ClangLazy${CMAKE_SHARED_LIBRARY_SUFFIX} -Xclang -add-plugin > -Xclang clang-lazy") > +

D18089: KLauncher: handle processes exiting without error

anthonyfieroni added a comment. +1 REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D18089 To: ahmadsamir, dfaure Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D18249: [datamodel] Rework items insert/remove

anthonyfieroni added a comment. I investigate on testing this patch, it does not fix systray issue, to discard it? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D18249 To: anthonyfieroni, davidedmundson, broulik, ngraham, mart, #plasma Cc:

D18425: [KUrlNavigatorPlacesSelector] Properly identify teardown action

anthonyfieroni added a comment. +1 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18425 To: broulik, #frameworks, dfaure Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D18249: [datamodel] Rework items insert/remove

anthonyfieroni updated this revision to Diff 49630. REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18249?vs=49469=49630 REVISION DETAIL https://phabricator.kde.org/D18249 AFFECTED FILES src/declarativeimports/core/datamodel.cpp To:

D18249: [datamodel] Rework items insert/remove

anthonyfieroni added inline comments. INLINE COMMENTS > anthonyfieroni wrote in datamodel.cpp:379 > Optimization is you have [5, 6, 8] then [5, 6, 8, 9] so it make only > beginInsertRows({}, 3, 3); > full replace (m_items[sourceName] = list.toVector();) > endInsertRows() > Same if you [5, 6] >

D18249: [datamodel] Rework items insert/remove

anthonyfieroni added inline comments. INLINE COMMENTS > davidedmundson wrote in datamodel.cpp:379 > I don't understand what you're changing here, can you provide a bit more > detail on the exact problem. Optimization is you have [5, 6, 8] then [5, 6, 8, 9] so it make only beginInsertRows({},

D18249: [datamodel] Rework items insert/remove

anthonyfieroni created this revision. anthonyfieroni added reviewers: davidedmundson, broulik, ngraham, mart, Plasma. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. anthonyfieroni requested review of this revision. REVISION SUMMARY I give a try to rework

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

anthonyfieroni accepted this revision. This revision is now accepted and ready to land. REPOSITORY R263 KXmlGui BRANCH arcpatch-D18204 REVISION DETAIL https://phabricator.kde.org/D18204 To: aacid, anthonyfieroni Cc: davidedmundson, anthonyfieroni, kde-frameworks-devel, michaelh, ngraham,

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

anthonyfieroni added inline comments. INLINE COMMENTS > kcheckaccelerators.cpp:87 > +static bool doCheckAccelerators = true; > + > QCoreApplication *app = QCoreApplication::instance(); We can cheet here if (!doCheckAccelerators) { return; } > kcheckaccelerators.cpp:114 > +

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

anthonyfieroni added inline comments. INLINE COMMENTS > kcheckaccelerators.cpp:97 > + > +if (!app->eventDispatcher()) { > +// We are called with event dispatcher being null when KXmlGui is > being loaded That's static member, you don't need instance. BTW can we check only for null

D18164: Review KateGotoBar

anthonyfieroni added inline comments. INLINE COMMENTS > loh.tar wrote in katedialogs.cpp:1130 > > It should be 120, > > Why? Have now read the doc, but without a new insight. > > > also you can have 2 separate delta members > > How and why? > you can either cumulatively add the delta values

D18164: Review KateGotoBar

anthonyfieroni added inline comments. INLINE COMMENTS > katedialogs.cpp:1130 > +// and with my mousepad was the experience also flawlessly > +if (delta > 100) { > +delta = 0; https://doc.qt.io/qt-5/qwheelevent.html#angleDelta It should be 120, also you can have 2

D18149: Share Plasma::Theme instances between multiple Svg and ColorScope

anthonyfieroni added a comment. Now in multi-thread environment will have problems especially on actualTheme(), no? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D18149 To: broulik, #plasma Cc: anthonyfieroni, kde-frameworks-devel, michaelh,

D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

anthonyfieroni added inline comments. INLINE COMMENTS > katepart5ui.rc:2 > > > Increase version REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17137 To: gregormi, #kate, #kdevelop Cc: anthonyfieroni, ngraham, cullmann, flherne, dhaumann, kwrite-devel,

D17459: SearchBar: Add Cancel button to stop long running tasks

anthonyfieroni added inline comments. INLINE COMMENTS > katesearchbar.cpp:860-865 > +d(this)->m_inputRange = inputRange; > +d(this)->m_workingRange = > m_view->doc()->newMovingRange(d(this)->m_inputRange); > +d(this)->m_replacement = replacement; > +d(this)->m_replaceMode =

D17459: SearchBar: Add Cancel button to stop long running tasks

anthonyfieroni added inline comments. INLINE COMMENTS > katesearchbar.cpp:89 > +ret = new KateSearchBarPrivate; > +d_func()->insert(foo, ret); > +} You can take another approach connect(foo, ::destroyed, d_func, [foo]() { delete d_func()->take(foo); }); >

D17459: SearchBar: Add Cancel button to stop long running tasks

anthonyfieroni added inline comments. INLINE COMMENTS > katesearchbar.cpp:83 > +typedef QHash > KateSearchBarPrivateHash; > +static KateSearchBarPrivateHash KateSearchBarPrivateData; > +Q_GLOBAL_STATIC(KateSearchBarPrivateHash, d_func) It's not needed, right? > katesearchbar.cpp:97-99 > +

D17459: SearchBar: Add Cancel button to stop long running tasks

anthonyfieroni added inline comments. INLINE COMMENTS > katesearchbar.h:153 > -bool find(SearchDirection searchDirection = SearchForward, const QString > *replacement = nullptr); > -int findAll(KTextEditor::Range inputRange, const QString *replacement); > It's exported class you

D17617: Display error instead of silently failing when asked to create folder that already exists

anthonyfieroni added inline comments. INLINE COMMENTS > ngraham wrote in knewfilemenu.cpp:916 > I don't see how that makes sense. What's wrong with the current approach of > expecting the jobs themselves to emit errors? Makes sense because it's done in one standard way, possible change in

D17617: Display error instead of silently failing when asked to create folder that already exists

anthonyfieroni added inline comments. INLINE COMMENTS > ngraham wrote in knewfilemenu.cpp:916 > It displays on the current tab, as expected. I still think it's better to emit errorMessage instead. Can you check it will be more difficult ? REPOSITORY R241 KIO REVISION DETAIL

D17617: Display error instead of silently failing when asked to create folder that already exists

anthonyfieroni added inline comments. INLINE COMMENTS > knewfilemenu.cpp:916 > job->uiDelegate()->setAutoErrorHandlingEnabled(true); > -KIO::FileUndoManager::self()->recordJob(KIO::FileUndoManager::Mkpath, > QList(), url, job); > +KJobWidgets::setWindow(job, m_parentWidget); >

D17441: tune editing actions for large number of small edits

anthonyfieroni added inline comments. INLINE COMMENTS > katetextblock.cpp:153 > // remember all ranges modified > -QSet changedRanges; > +std::vector changedRanges; > foreach (TextCursor *cursor, m_cursors) { You can use unordered_set, so you don't need

D15763: Set correct image attributes on directory thumbnail

anthonyfieroni added a comment. To 18.12 ? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15763 To: broulik, #frameworks, dfaure, anthonyfieroni, jtamate Cc: ngraham, kde-frameworks-devel, kfm-devel, alexde, sourabhboss, feverfew, michaelh, spoorun,

D17398: [xscreensaverpoller] Flush after reset screensaver

This revision was automatically updated to reflect the committed changes. Closed by commit R274:de990fe48ff4: [xscreensaverpoller] Flush after reset screensaver (authored by anthonyfieroni). Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. REPOSITORY R274

D17325: Fix leak in kemoticons

anthonyfieroni added inline comments. INLINE COMMENTS > kemoticonstheme.cpp:57 > d = new KEmoticonsThemeData; > -d->provider = p; > +d->provider.reset(p); > } I've rethink it, this line can be a problem if pointer is not owned by us. Did you know consumer of the KEmoticonsTheme?

D17325: Fix leak in kemoticons

anthonyfieroni added inline comments. INLINE COMMENTS > kemoticonstheme.cpp:32 > ~KEmoticonsThemeData(); > KEmoticonsProvider *provider = nullptr; > }; Make it shared pointer, KEmoticonsTheme::KEmoticonsTheme(const KEmoticonsTheme ) initialize provider with raw other pointer,

D17078: Make it possible to use KAboutData/License/Person from QML

anthonyfieroni added a reviewer: dfaure. anthonyfieroni added a comment. You can't change parameters in methods or constructors, it's a BIC. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D17078 To: apol, #frameworks, dfaure Cc: anthonyfieroni,

D16643: Correct the accept flag of the event object on DragMove

anthonyfieroni added inline comments. INLINE COMMENTS > DeclarativeDropArea.cpp:97 > if (event->pos() == m_oldDragMovePos) { > -event->setAccepted(false); > return; Change it to accept() and try it. REPOSITORY R296 KDeclarative REVISION DETAIL

D16434: Fix keyboard layout change notifications

anthonyfieroni added inline comments. INLINE COMMENTS > kglobalaccel_x11.cpp:198-208 > +typedef union { > + /* All XKB events share these fields. */ > + struct { > + uint8_t response_type; > + uint8_t xkbType; > + uint16_t sequence; > +

D16311: RFC: [KFilePlacesView] Use asynchronous KIO::FileSystemFreeSpaceJob

anthonyfieroni added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kfileplacesview.cpp:257 > You can make lambda mutable in there you can modify m_freeSpaceInfo without > declare it mutable No ignore it, it does not work. REPOSITORY R241 KIO REVISION DETAIL

D16311: RFC: [KFilePlacesView] Use asynchronous KIO::FileSystemFreeSpaceJob

anthonyfieroni added inline comments. INLINE COMMENTS > kfileplacesview.cpp:256 > +(!info.lastUpdated.isValid() || > info.lastUpdated.secsTo(QDateTime::currentDateTimeUtc()) > 60)) { > +info.job = KIO::fileSystemFreeSpace(url); > +

D16366: Fix crash with icon dialog

anthonyfieroni added a comment. https://phabricator.kde.org/source/kiconthemes/browse/master/src/kicondialog.cpp;fea52a4f204fd44d9a45e78a663964c326687e33$419 It should be const QString fileName2 = path2.mid(path2.lastIndexOf(QLatin1Char('/')) + 1); no? REPOSITORY R302

D15763: Set correct image attributes on directory thumbnail

anthonyfieroni added a comment. +1 REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15763 To: broulik, #frameworks, dfaure, anthonyfieroni, jtamate Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros,

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

anthonyfieroni added inline comments. INLINE COMMENTS > main.cpp:217 > +int posOfNonNumeric = word.indexOf('-', 2); > +if ((posOfNonNumeric < -1) || ((posOfNonNumeric + 1) > == word.length())) { > +stream << "Malformed

D15818: [Exe Thumbnailer] Ignore depth > 32

anthonyfieroni accepted this revision. This revision is now accepted and ready to land. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15818 To: broulik, anthonyfieroni Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef,

D15402: [Thumbnails] Paint larger "one thumbnail" tile only when needed

anthonyfieroni added a comment. Looks good +1 REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15402 To: broulik, #dolphin, ngraham Cc: anthonyfieroni, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros,

D13627: [KSharedDataCache] Assume lock before flush changes

anthonyfieroni added a comment. It looks can happen https://www.reddit.com/r/kde/comments/9hovrv/ive_been_having_this_randomly_icons_appears_to_be/ But i'm not sure that patch can handle it. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13627 To:

D15623: Finish the notification before removing it from the hash

anthonyfieroni added a comment. LGTM +1 REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D15623 To: jtamate, #frameworks, sitter Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D15583: [Balooctl] fix directory parent check

anthonyfieroni added a comment. In Qt 5.9 (if i remember correct) was introduced to not have trailing '/' so, QDir::separator should not be used, so better get folder as copy for (QString folder : folders) { if (!folder.endsWith(QLatin1Char('/')) { folder +=

D15417: [AppImage Thumbnailer] Avoid creating QTemporaryFile

anthonyfieroni added a comment. Maybe it's better to make new code conditional to version that satisfy appimage_read_file_into_buffer_following_symlinks REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15417 To: broulik, #frameworks, TheAssassin, anthonyfieroni

D15417: [AppImage Thumbnailer] Avoid creating QTemporaryFile

anthonyfieroni requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15417 To: broulik, #frameworks, TheAssassin, anthonyfieroni

D15417: [AppImage Thumbnailer] Avoid creating QTemporaryFile

anthonyfieroni added a comment. https://phabricator.kde.org/source/kio-extras/browse/master/thumbnail/CMakeLists.txt$10 It should be specified the version that contains appimage_read_file_into_buffer_following_symlinks REPOSITORY R320 KIO Extras REVISION DETAIL

D14006: Fix WebDAV destination header on COPY and MOVE operations

anthonyfieroni added a comment. +1 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14006 To: dantti, dfaure, #frameworks, #dolphin Cc: anthonyfieroni, ngraham, kde-frameworks-devel, michaelh, bruns

D15328: kfilewidget: convert connect syntax

anthonyfieroni added inline comments. INLINE COMMENTS > jtamate wrote in kfilewidget.cpp:1225 > I'm sorry but I don't understand your comment. > m_connEditTextChanged is created in the constructor, line 585. > Afterwards it is disconnected and reconnected in: > setDummyHistoryEntry,

D15328: kfilewidget: convert connect syntax

anthonyfieroni added inline comments. INLINE COMMENTS > kfilewidget.cpp:1225 > // the KDirOperator's view-selection in there > -QObject::disconnect(locationEdit, SIGNAL(editTextChanged(QString)), > -q, SLOT(_k_slotLocationChanged(QString))); > +

D15317: trash: Fix directorysizes cache parsing

anthonyfieroni added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D15317 To: volkov, #frameworks, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15180: kioexecd: watch for creations or modifications of the temporary files

anthonyfieroni added inline comments. INLINE COMMENTS > jtamate wrote in kioexecd.cpp:65 > There is a slightly problem: QStandardPaths::CacheLocation is application > dependent, and their values doesn't match here: > kioexec: /home/jtorres/.cache/kioexec/ > kioexecd:

D15180: kioexecd: watch for creations or modifications of the temporary files

anthonyfieroni added a comment. m_timer.start(predefinedTimeout) should be m_timer.start() in 2 places, but let's see @elvisangelaccio and @dfaure comments. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D15180 To: jtamate, #frameworks, broulik, ngraham, dfaure,

D15180: kioexecd: watch for creations or modifications of the temporary files

anthonyfieroni added inline comments. INLINE COMMENTS > kioexecd.cpp:53 > connect(m_watcher, ::deleted, this, ::slotDeleted); > +m_timer.setSingleShot(true); > +connect(_timer, ::timeout, this, > ::slotCheckDeletedFiles); Also add interval here, setInterval, in other place just

D15180: kioexecd: watch for creations or modifications of the temporary files

anthonyfieroni added inline comments. INLINE COMMENTS > kioexecd.cpp:80 > +{ > +if (!m_deleted.contains(path)) { > +return; Contains should be also in the guard. > kioexecd.h:40 > virtual ~KIOExecd(); > +void clearDir(const QString ); > Unused? REPOSITORY R241 KIO

D15180: kioexecd: watch for creations or modifications of the temporary files

anthonyfieroni added inline comments. INLINE COMMENTS > kioexecd.cpp:134 > +QDir(parentDir).removeRecursively(); > +it=m_deleted.erase(it); > +} Also for loop should looks like: for (it = begin(); it != end();) { if () { it =

D15180: kioexecd: watch for creations or modifications of the temporary files

anthonyfieroni added inline comments. INLINE COMMENTS > kioexecd.cpp:122 > +m_deleted_mutex.unlock(); > +QTimer::singleShot(31000, [this]() { // 31s > +m_deleted_mutex.lock(); Better to me, make a class variable single shot timer, then when you add in deleted start it if not,

D15180: kioexecd: watch for creations or modifications of the temporary files

anthonyfieroni added a comment. I'm unhappy with that stop watching is on exit == 0, so when it's not, somehow, containers will continue to grow, it'll result in higher memory usage and slower performance. So stop watching should not depend on process return code, also same command should

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

anthonyfieroni added inline comments. INLINE COMMENTS > anthonyfieroni wrote in krun.cpp:1319 > You cannot remove it, since it can be a local file, remote file with scheme > that differs http and should be redirect then. So you can make it like that > > if

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

anthonyfieroni added inline comments. INLINE COMMENTS > krun.cpp:1319 > -// Update our URL in case of a redirection > -setUrl(job->url()); > - You cannot remove it, since it can be a local file, remote file with scheme that differs http and should be redirect then. So you can

D15071: Don't draw frames and shadows around images with transparency

anthonyfieroni added a comment. In D15071#315839 , @broulik wrote: > Why did the `ThumbCreator` frame flag get deprecated in the first place? https://git.reviewboard.kde.org/r/129921/ maybe because it's deprecated before thumbnail got

D13627: [KSharedDataCache] Assume lock before flush changes

anthonyfieroni added a comment. I've test it too, but after kernel upgrade i don't notice it anymore, it can be a kernel regression, it still looks valuable to you, @mpyne ? REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13627 To: anthonyfieroni,

D15013: balootctl: fix 396535

anthonyfieroni added a comment. Do not use QDir::separator if (!folder.endsWith(QLatin1Char('/')) { folder += QLatin1Char('/'); } REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15013 To: jausmus Cc: anthonyfieroni, kde-frameworks-devel, #baloo,

D6473: Crash when replacing new lines with spaces

anthonyfieroni added a comment. I'm aware of that it will fix the crash but functionally still will be missing, but i'll give a try. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D6473 To: jsalatas, #ktexteditor, dhaumann Cc: kde-frameworks-devel, mwolff,

D6473: Crash when replacing new lines with spaces

anthonyfieroni added a comment. About me when line(...) is accessed it should check for nullptr as well if (auto l = line(a)) { return l->accessor(); }

D14826: inline note interface wip #2

anthonyfieroni added inline comments. INLINE COMMENTS > katerenderer.cpp:765 > +// Draw inline notes > +auto inlineNotes = m_view->inlineNotes(range->line()); > +foreach (const KTextEditor::InlineNote& inlineNote, inlineNotes) { const > katerenderer.cpp:766 > +

D14661: Force reevaluation of Predicates if interfaces are removed

anthonyfieroni added a reviewer: apol. anthonyfieroni added a subscriber: apol. anthonyfieroni added a comment. Since solid does not have a maintainer, you can wait for @apol or @broulik or ship it before 5.50 tagging. REPOSITORY R245 Solid REVISION DETAIL

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni abandoned this revision. anthonyfieroni added a comment. https://phabricator.kde.org/D14661 REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure, bruns Cc: ngraham, bcooksley, bruns, kde-frameworks-devel,

D14661: Force reevaluation of Predicates if interfaces are removed

anthonyfieroni added a comment. Ok, +1 then. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D14661 To: bruns, #frameworks, broulik, ngraham Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D14661: Force reevaluation of Predicates if interfaces are removed

anthonyfieroni added inline comments. INLINE COMMENTS > udisksmanager.cpp:239 > +emit deviceRemoved(udi); > +emit deviceAdded(udi); > } Your code seems to work, but i don't get it - why device is added again? How predicate will work on removed device? REPOSITORY R245

D14689: [KFileItem] Don't read directory comment on slow mounts

anthonyfieroni added inline comments. INLINE COMMENTS > dfaure wrote in kfileitem.cpp:816 > Shouldn't you check isSlow() at the end of the condition, or at least after > isDir()? This way it wouldn't be checked for non-directories (you say it > takes time...). +1, at end will be better.

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. In D13869#303905 , @bruns wrote: > And thinking about it, this very likely makes your code wrong. Have you tried mounting a filesystem again after unmounting it, **using via any mean implemented via

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. About loop device it should present as well http://storaged.org/doc/udisks2-api/2.6.3/gdbus-org.freedesktop.UDisks2.Loop.html REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure, bruns Cc:

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. Since you make question to new code I make for old one, did you why Device interfaces is ever used, it's look wrong and they should be empty, after all my tests, it's easy to see that they are never empty. REPOSITORY R245 Solid REVISION DETAIL

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. I'm using too. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. For 5.49? REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure Cc: bcooksley, bruns, kde-frameworks-devel, michaelh, ngraham

D14360: Remove custom icon selection for trash

anthonyfieroni added inline comments. INLINE COMMENTS > kfileplaceeditdialog.cpp:220 > +{ > +return m_iconButton; > +} This will produce error on strict level it should be m_iconButton != nullptr REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14360 To: shubham,

D14162: Figure out the escaped path list on kconfig

anthonyfieroni added inline comments. INLINE COMMENTS > kconfiggroup.cpp:176 > +last = p + 1; > +} else if (data[p] == QLatin1Char('\\')) { > +escapedLast = true; It should be p < data.size() && data[p] == '\\' or you will access not owned memory. Better is

D14308: [Exe Thumbnailer] Improve icon selection algorithm

anthonyfieroni added inline comments. INLINE COMMENTS > bruns wrote in icoutils_common.cpp:33 > @anthonyfieroni Why? Comparing doubles should be done by epsilon not by <= 0.0 REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D14308 To: broulik, #frameworks, dfaure,

D14308: [Exe Thumbnailer] Improve icon selection algorithm

anthonyfieroni added inline comments. INLINE COMMENTS > icoutils_common.cpp:33 > +qreal delta = width - desiredWidth; > +delta = (delta >= 0.0 ? delta : -delta * 2 ); // Penalize for scaling up > + It should be qFuzzyIsNull no? REPOSITORY R320 KIO Extras REVISION DETAIL

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. Ping, anything unclear ? REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure Cc: bcooksley, bruns, kde-frameworks-devel, michaelh, ngraham

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. Device(udi).interfaces() ("org.freedesktop.UDisks2.Loop", "org.freedesktop.UDisks2.Block") interfaces ("org.freedesktop.UDisks2.Filesystem") REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck,

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added inline comments. INLINE COMMENTS > bruns wrote in udisksmanager.cpp:220 > This does not answer my question. What I need from you is: > > - which interfaces are in `interfaces` > - which interfaces are in `device.interfaces()` > > so the **exact** case where the patched

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added inline comments. INLINE COMMENTS > bruns wrote in udisksmanager.cpp:220 > This does not answer my question. What I need from you is: > > - which interfaces are in `interfaces` > - which interfaces are in `device.interfaces()` > > so the **exact** case where the patched

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. Moundting by cdemu also works as expected, not without patch as is described in bug report. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure Cc: bcooksley, bruns, kde-frameworks-devel,

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. Performing working tests - mount -o loop xxx.iso /mnt/xxx - mount xxx/iso /mnt/xxx - Add Android phone - Add second phone, first become inactive, second works REPOSITORY R245 Solid REVISION DETAIL

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni updated this revision to Diff 38153. anthonyfieroni added a comment. First tests looks good, i'll make more later REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13869?vs=37129=38153 REVISION DETAIL https://phabricator.kde.org/D13869

D14253: avoid memory leak over sftp

anthonyfieroni added a comment. It's deleted in https://phabricator.kde.org/source/kio/browse/master/src/core/slaveinterface_p.h$47 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14253 To: jtamate, dfaure, #frameworks, ngraham Cc: anthonyfieroni, apol,

D13808: Fix KMainWindow saving incorrect widget settings

anthonyfieroni added inline comments. INLINE COMMENTS > maxrd2 wrote in kmainwindow_unittest.cpp:267 > Retried with all of these... none of them causes the failure, only closing > the window manually cause it > > QApplication::postEvent(mw, new QDeferredDeleteEvent); // qpa/window > manager

D13570: Add workaround for labels with word-wrapping

anthonyfieroni added inline comments. INLINE COMMENTS > kcollapsiblegroupbox.cpp:307-308 > +if (label->wordWrap()) { > +toggle(); > +toggle(); > +} Did you know why it should be called twice? Does one later toggle invoke will fix it? REPOSITORY R236

D13627: [KSharedDataCache] Assume lock before flush changes

anthonyfieroni added a comment. @mpyne , i'm unsure about file deletion and sync other than that it looks correct. Finding best lock algorithm can be *potential* problem, but surely it's not my best. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D13627 To:

D13627: [KSharedDataCache] Assume lock before flush changes

anthonyfieroni updated this revision to Diff 37343. REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13627?vs=36742=37343 REVISION DETAIL https://phabricator.kde.org/D13627 AFFECTED FILES src/lib/caching/kshareddatacache.cpp To: anthonyfieroni,

D13627: [KSharedDataCache] Assume lock before flush changes

anthonyfieroni retitled this revision from "[KIconThemes] Isolate private data from race conditions" to "[KSharedDataCache] Assume lock before flush changes". anthonyfieroni edited the summary of this revision. anthonyfieroni added a reviewer: mpyne. anthonyfieroni changed the repository for this

D13627: [KIconThemes] Isolate private data from race conditions

anthonyfieroni added a comment. So the probably is there, i'll investigate, thanks. REPOSITORY R302 KIconThemes REVISION DETAIL https://phabricator.kde.org/D13627 To: anthonyfieroni, davidedmundson, dfaure, #frameworks, hein Cc: mpyne, hein, kde-frameworks-devel, michaelh, ngraham,

D13627: [KIconThemes] Isolate private data from race conditions

anthonyfieroni added a comment. https://phabricator.kde.org/source/kiconthemes/browse/master/src/kiconloader.cpp$603 So the problem maybe not here as IconLoader loads icons but in icon writer @davidedmundson , @dfaure who writes icon-cache.kcache in ~/.cache ? REPOSITORY R302

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. I'm aware that does not an idea, what is the purpose of Predicate. What should i search in Solid::Predicate::matches? REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure Cc: bruns,

D13627: [KIconThemes] Isolate private data from race conditions

anthonyfieroni added a reviewer: hein. anthonyfieroni added a subscriber: hein. anthonyfieroni added a comment. It happen again, so it's not a threading issue @hein when you hover grouped task in taskmanager end up with broken icons, sometimes, kcache in ~/.cache is generated at same that

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. So about docs when interface is added it will notify again for mounting, i think that's the idea behind interfaces. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure Cc: bruns,

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added a comment. So if KFilePlacesModel wants to hide, i don't see how this will break things, it only notify when udisk2 loose interface to mounted fs. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D13869 To: anthonyfieroni, broulik, cfeck, dfaure

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni added inline comments. INLINE COMMENTS > bruns wrote in udisksmanager.cpp:220 > This code needs some restructuring, and with the additional conditions, some > comments ... > > 1. `if (udi.isEmpty()) return;` > 2. you are whitelisting a lot of conditions (`... || .. || ...`) -

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni updated this revision to Diff 37129. anthonyfieroni added a comment. 1. Return early when path is empty 2. Don't check for empty interfaces REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13869?vs=37110=37129 REVISION DETAIL

D13869: [solid] Notify when interface to mounted fs is lost

anthonyfieroni updated this revision to Diff 37110. anthonyfieroni added a comment. Add loop interface REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13869?vs=37108=37110 REVISION DETAIL https://phabricator.kde.org/D13869 AFFECTED FILES

<    1   2   3   4   5   6   7   >