D27152: Introduce FilesystemEntry class

hallas marked an inline comment as done. hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabhandling.cpp:374 > The `id()` method and members are still part of the FstabEntry, where they do > not belong. > > The UDIs go through some further mangling in

D27152: Introduce FilesystemEntry class

hallas marked an inline comment as done. hallas added a comment. Hi @bruns - did you have a chance to go through this patch again? Am I missing anything to move on with this? REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D27152 To: hallas, #frameworks, bruns, meven

D27152: Introduce FilesystemEntry class

hallas marked 4 inline comments as done. hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabhandling.cpp:374 > For now just keep it here, I can see now benefit of making it public (even > unexported). > > And thanks for baring with my nitpicking ;-), this now is pretty much

D27152: Introduce FilesystemEntry class

hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabhandling.cpp:374 > Why is the id() a property of the entry now? > > - it does not correspond to anything from the fstab/mtab > - it is only used here to generate the key for the cache > > Please move the id generation back

D27152: Introduce FilesystemEntry class

hallas updated this revision to Diff 76990. hallas marked 7 inline comments as done. hallas added a comment. Review comments REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27152?vs=76898=76990 BRANCH introduce_filesystem_entry REVISION DETAIL

D27152: Introduce FilesystemEntry class

hallas added inline comments. INLINE COMMENTS > bruns wrote in filesystem_entry.cpp:31 > This is a behavioral change: > > - overlayfs is no longer handled > - fuse.cryfs is no longer handled > > - encfs gets an extra '@', and the prefix is changed from to > > The identifier is API, don't

D27152: Introduce FilesystemEntry class

hallas updated this revision to Diff 76898. hallas marked 9 inline comments as done. hallas added a comment. Addressed review comments REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27152?vs=75961=76898 BRANCH introduce_filesystem_entry REVISION DETAIL

D27152: Introduce FilesystemEntry class

hallas added a comment. @bruns - thanks a lot for the feedback, I have updated the patch with your suggestions. INLINE COMMENTS > bruns wrote in filesystem_entry.cpp:77 > Why don't you just keep the string? It was merely meant as an optimization, I don't know if it makes sense - I'll

D27152: Introduce FilesystemEntry class

hallas added a comment. Hi @bruns - I have updated this patch with the changes you requested and would really like your feedback :) REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D27152 To: hallas, #frameworks, bruns, meven Cc: kde-frameworks-devel, LeGast00n,

D27152: Introduce FilesystemEntry class

hallas added inline comments. INLINE COMMENTS > meven wrote in filesystem_entry.cpp:47 > This sound sane to me. > Ideally we would store only the string in case of an unknown fs or the enum > but not both for a same entry, the choice could be done in ctor. I have added a new enum class type

D27504: WIP: RFC: smb faster copy to local

hallas added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kio_smb_dir.cpp:49 > Segment does not free its memory. Why not use QByteArray or similar with > automatic storage allocation? Yes, you could (and should ;) ) use standard C++ for this, i.e.: std::unique_ptr buf; buf =

D27152: Introduce FilesystemEntry class

hallas updated this revision to Diff 75961. hallas marked 17 inline comments as done. hallas added a comment. Updated patch with review comments REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27152?vs=74977=75961 BRANCH introduce_filesystem_entry

D27152: Introduce FilesystemEntry class

hallas added inline comments. INLINE COMMENTS > meven wrote in filesystem_entry.cpp:47 > It would be great to expose an enum (KIO's KFileSystemType does this a > little). > But given the number of filesystems this days, it may be overcomplicated. Yeah I agree ;) But I would really like for

D27152: Introduce FilesystemEntry class

hallas created this revision. hallas added reviewers: Frameworks, bruns, meven. hallas added a project: Frameworks. hallas requested review of this revision. REVISION SUMMARY Introdue a dedicated type for representing a FilesystemEntry. A FilesystemEntry is either mounted by the system or can

D26600: Refactor fstab handling

hallas added a comment. In D26600#596631 , @bruns wrote: > and this is definitely too much code being moved around. Please split this up into multiple reviews. > > You should likely start with the introduction of the FilesystemEntry class,

D26600: Refactor fstab handling

hallas added a comment. In D26600#596533 , @bruns wrote: > Moving existing code to new files does not make you the copyright owner. Yes, sorry about that - I need to fix that. This is just my editor, when I create new files this is the

D21235: Add handling of fuseiso filesystem type

hallas added a comment. I have refactored the fstab handling to make supporting fuseiso _much_ simpler - so please take a look at D26600 - once that is merged I will push a new review of this patch. REPOSITORY R245 Solid REVISION DETAIL

D26600: Refator fstab handling

MMENTS > call_system_command.cpp:1 > +/*** > + * Copyright (C) 2019 by David Hallas * I think the Copyright notice needs to include the original authors of this function > filesystem_

D26600: Refator fstab handling

hallas created this revision. hallas added reviewers: Frameworks, bruns, meven. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. hallas requested review of this revision. REVISION SUMMARY Refactor fstab handling Splits the fstab/mtab handling code into

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas added a comment. Sorry for breaking the build :/ @kossebau - thanks for fixing it so quickly! I think your fix looks fine :D REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23205 To: hallas, davidedmundson, broulik, mpyne Cc: kossebau, bcooksley,

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas closed this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23205 To: hallas, davidedmundson, broulik, mpyne Cc: mpyne, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21235: Add handling of fuseiso filesystem type

hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabhandling.cpp:301 > The API is totally awkward. > > You end up with something significantly cleaner when you create a new class > for it: > > QStringMultiHash m_mtabCache; > QHash m_mtabFstypeCache; > bool

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas added a comment. @davidedmundson - ping ;) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23205 To: hallas, davidedmundson, broulik Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21235: Add handling of fuseiso filesystem type

hallas added a comment. @bruns - ping ;) REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21235 To: hallas, bruns, ngraham Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25125: Use KListOpenFilesJob for retrieving apps blocking unmount

hallas added a comment. In D25125#558043 , @anthonyfieroni wrote: > Set QT_PLUGIN_PATH to you local path with plugin after that set the system path for others > `QT_PLUGIN_PATH=/local/path:/system/path executable` Thanks for the info

D25125: Use KListOpenFilesJob for retrieving apps blocking unmount

hallas added a comment. Note that I haven't tested this patch locally because I don't know how to "run" the locally compiled devicenotifier. I can see that I get a `plasma_engine_devicenotifications.so` library from building the project, but I don't know how to "run" it, do we have a cli

D25125: Use KListOpenFilesJob for retrieving apps blocking unmount

hallas created this revision. hallas added reviewers: Frameworks, broulik, bruns. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. hallas requested review of this revision. REVISION SUMMARY Use the KListOpenFilesJob class from KCoreAddons to retrieve the applications

D21235: Add handling of fuseiso filesystem type

hallas added a comment. @bruns - ping :) I have updated this patch with the changes you requested, I hope you are ok with it now. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21235 To: hallas, bruns, ngraham Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2,

D21235: Add handling of fuseiso filesystem type

hallas marked an inline comment as done. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D21235 To: hallas, bruns, ngraham Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21235: Add handling of fuseiso filesystem type

hallas updated this revision to Diff 68472. hallas added a comment. Reorder functions to make diff smaller REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21235?vs=68324=68472 BRANCH add_handling_of_fuseiso (branched from master) REVISION DETAIL

D21235: Add handling of fuseiso filesystem type

hallas marked an inline comment as done. hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabhandling.cpp:301 > Please keep these functions at their current position, just adds unnecessary > noise in the diff. Good point :) REPOSITORY R245 Solid REVISION DETAIL

D21235: Add handling of fuseiso filesystem type

hallas marked 3 inline comments as done. hallas added a comment. @bruns - I have now refactored the patch so that it uses the `getmntent` functions for parsing the mtab file, so I think this patch is pretty much ready for a serious review ;) Also - please take a look at the strings that

D21235: Add handling of fuseiso filesystem type

hallas updated this revision to Diff 68324. hallas added a comment. Rewrite to use the getmntent function for parsing the mtab file REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21235?vs=68230=68324 BRANCH add_handling_of_fuseiso (branched from master)

D21235: Add handling of fuseiso filesystem type

hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabdevice.cpp:56 > fuseiso itself uses setmntent/addmntent/endmntent, so I see no reason > getmntent should not work. > Most importantly, the same implementation should be used for writing and > reading. > >

D21235: Add handling of fuseiso filesystem type

hallas added a comment. Currently you have the 'unmount' action if you right click on the device in dolphin, but it cannot unmount, so should we hide it? Or should we fix it so that it can actually unmount? INLINE COMMENTS > fstabdevice.cpp:56 > + > +QString

D21235: Add handling of fuseiso filesystem type

hallas updated this revision to Diff 68230. hallas added a comment. Implemented parsing of the fuseiso mtab file REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21235?vs=58147=68230 BRANCH add_handling_of_fuseiso (branched from master) REVISION DETAIL

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas added a comment. @davidedmundson ping :) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23205 To: hallas, davidedmundson, broulik Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24434: Fix include path to kjob.h

hallas added a comment. In D24434#542440 , @dfaure wrote: > Argh, I retagged, but this wasn't pushed yet. > Landing and retagging again... Thanks a lot and sorry for the screwups :D REPOSITORY R244 KCoreAddons REVISION DETAIL

D24434: Fix include path to kjob.h

hallas added a comment. @dfaure - I just found that if you include `KListOpenFilesJob` from e.g. Dolphin then it fails because it cannot include `jobs/kjob.h` :( It would be really nice to have this commit in the coming Frameworks release, do you think that is possible? Sorry for

D24434: Fix include path to kjob.h

hallas created this revision. hallas added reviewers: dfaure, meven. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. hallas requested review of this revision. REVISION SUMMARY Fix include path to kjob.h. When kcoreaddons is installed all the headers files

D24389: Fix klistopenfilesjob header file not being installed

hallas added a comment. In D24389#541644 , @dfaure wrote: > `git tag` doesn't show any 5.63.0-rc* tag yet, so it hasn't been tagged, so this commit will be included. > > (I do that on the first Saturday of the month) Phew :) Glad

D24389: Fix klistopenfilesjob header file not being installed

hallas added a comment. In D24389#541539 , @ngraham wrote: > Please land this ASAP so it gets into Frameworks 5.63. Landed it now, is there anything else I need to do so that it makes the 5.63 release? REPOSITORY R244 KCoreAddons

D24389: Fix klistopenfilesjob header file not being installed

hallas closed this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D24389 To: hallas, dfaure, meven, ngraham Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24389: Fix klistopenfilesjob header file not being installed

hallas created this revision. hallas added reviewers: dfaure, meven. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. hallas requested review of this revision. REVISION SUMMARY The header file KListOpenFilesJob and klistopenfilesjob.h was not being

D21235: Add handling of fuseiso filesystem type

hallas added a comment. In D21235#536576 , @hallas wrote: > In D21235#535951 , @bruns wrote: > > > In D21235#535861 , @hallas wrote: > > > > > I have

D21235: Add handling of fuseiso filesystem type

hallas added a comment. In D21235#535951 , @bruns wrote: > In D21235#535861 , @hallas wrote: > > > I have been resurrecting this patch again :) and have run into an issue I need some guidance on. To

D21235: Add handling of fuseiso filesystem type

hallas added a comment. I have been resurrecting this patch again :) and have run into an issue I need some guidance on. To be able to parse the `~/.mtab.fuseiso` file I would like to use the `KMountPoint` class, but this class currently resides in KIO which Solid doesn't depend on. But,

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas added a comment. @davidedmundson - ping :) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23205 To: hallas, davidedmundson, broulik Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed

hallas closed this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23884 To: hallas, dfaure Cc: dhaumann, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed

hallas marked 2 inline comments as done. hallas added inline comments. INLINE COMMENTS > dfaure wrote in klistopenfilesjobtest_unix.cpp:67 > Please use QSKIP instead, otherwise the test output looks like the test > passed, rather than was skipped. Good point, I didn't know that :) REPOSITORY

D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed

hallas updated this revision to Diff 65937. hallas added a comment. Use QSKIP to skip tests REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23884?vs=65870=65937 BRANCH fix_unittest_if_lsof_is_not_installed REVISION DETAIL

D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed

hallas updated this revision to Diff 65870. hallas added a comment. Use QStandardPaths::findExecutable to locate lsof REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23884?vs=65869=65870 BRANCH fix_unittest_if_lsof_is_not_installed REVISION DETAIL

D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed

hallas added inline comments. INLINE COMMENTS > dhaumann wrote in klistopenfilesjobtest_unix.cpp:34 > Can't you simply use QStandardPaths::findExecutable()? > > https://doc.qt.io/qt-5/qstandardpaths.html#findExecutable Yes, this seems to do exactly what we need :) > ngraham wrote in

D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed

hallas created this revision. hallas added a reviewer: dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. hallas requested review of this revision. REVISION SUMMARY The KListOpenFilesJob unit test on Unix expected lsof to be installed, but the KDE

D21760: Add KListOpenFilesJob

hallas closed this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma Cc: meven, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65856. hallas added a comment. Updated @since REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65500=65856 BRANCH add_list_processes_with_open_files (branched from master) REVISION DETAIL

D21760: Add KListOpenFilesJob

hallas added a comment. In D21760#526652 , @dfaure wrote: > This is good to go in :-) > > If you push it today it'll indeed be in 5.62, to be tagged tomorrow, otherwise we'll need to adjust the @since tag ;-) I think i prefer waiting

D21760: Add KListOpenFilesJob

hallas added inline comments. INLINE COMMENTS > dfaure wrote in klistopenfilesjobtest_unix.cpp:34 > (minor) we never check that `new` succeeded, in Qt code. > > The reasoning is that on desktop systems, with swap enabled, before the swap > is exhausted, the user will have given up and rebooted

D21760: Add KListOpenFilesJob

hallas edited the summary of this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma Cc: meven, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65500. hallas marked 7 inline comments as done. hallas added a comment. Review comments REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65494=65500 BRANCH add_list_processes_with_open_files (branched from

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65494. hallas added a comment. Review comments, renamed the files to match the class name REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65330=65494 BRANCH add_list_processes_with_open_files (branched

D21760: Add KListOpenFilesJob

hallas marked an inline comment as done. hallas added a comment. In D21760#525842 , @dfaure wrote: > Yes, the filenames should match the classname, obviously :) > > I did a deeper review of the KJob usage and I have two more comments, sorry

D21760: Add KListOpenFilesJob

hallas added inline comments. INLINE COMMENTS > klistopenfiles.h:23 > + > +#ifndef KLISTOPENFILES_H > +#define KLISTOPENFILES_H Should these files be renamed to klistopenfilesjob (along with tests etc.) now that is what the class is called? REPOSITORY R244 KCoreAddons REVISION DETAIL

D21760: Add KListOpenFilesJob

hallas added a comment. In D21760#524860 , @dfaure wrote: > Given that the namespace doesn't contain anything else anymore, I would just get rid of it, and provide a single class, KListOpenFilesJob. Done :) REPOSITORY R244 KCoreAddons

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65330. hallas added a comment. Removed KListOpenFiles namespace and renamed ListOpenFilesJob to KListOpenFilesJob REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65284=65330 BRANCH

D21760: Add KListOpenFilesJob

hallas retitled this revision from "Add KListOpenFiles::ListOpenFilesJob" to "Add KListOpenFilesJob". hallas edited the summary of this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns,

D21760: Add KListOpenFiles::ListOpenFilesJob

hallas retitled this revision from "Add KListOpenFiles::listProcessesWithOpenFiles" to "Add KListOpenFiles::ListOpenFilesJob". hallas edited the summary of this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik,

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added inline comments. INLINE COMMENTS > dfaure wrote in klistopenfilestest_unix.cpp:83 > I usually just use "/does/not/exist" as a path ;-) > > This might actually be better because Windows has weird race conditions with > the filesystem stuff. This test is unix only anyway so there

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas updated this revision to Diff 65284. hallas marked 3 inline comments as done. hallas added a comment. Review comments REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65237=65284 BRANCH add_list_processes_with_open_files (branched from

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added a comment. I have added a minimal Windows implementation which always emits an error, along with a unit test. Please review it thoroughly and then I think it is ready to land :) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas,

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas updated this revision to Diff 65237. hallas marked 2 inline comments as done. hallas added a comment. Review comments. Added minimal Windows implementation which basically always reports failure with the error code Unsupported. REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added a comment. One thing, when this is ready to land I will address the Windows support so that we do not get broken builds :) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns,

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas updated this revision to Diff 64973. hallas marked 2 inline comments as done. hallas added a comment. Review comments REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=64911=64973 BRANCH add_list_processes_with_open_files (branched from

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added a comment. @dfaure - Overall, what do you think about the approach of subclassing KJob? Did it turn out like you had thought? And is this the solution we should go with, or was one of the other solutions better? INLINE COMMENTS > dfaure wrote in klistopenfiles.cpp:29 > I

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas edited the test plan for this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23205 To: hallas, davidedmundson, broulik Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas updated this revision to Diff 64914. hallas added a comment. Add bug reference REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23205?vs=64913=64914 BRANCH optimize_kprocesslist_processinfo REVISION DETAIL https://phabricator.kde.org/D23205

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas marked an inline comment as done. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D23205 To: hallas, davidedmundson, broulik Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas updated this revision to Diff 64913. hallas marked an inline comment as done. hallas added a comment. Fixed review comments, rebased. REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23205?vs=63873=64913 BRANCH optimize_kprocesslist_processinfo

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added inline comments. INLINE COMMENTS > klistopenfiles.cpp:44 > +if (!path.exists()) { > +job->setError(KJob::UserDefinedError); > +job->emitResult(); Should I also set an error string here? And what about defining custom error codes, is that how we

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added a comment. @meven , so I finally managed to rewrite this patch to use KJob instead. Please take a look at it again and see if this is better approach :) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik,

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas updated this revision to Diff 64911. hallas added a comment. Rewrote the code to use KJob REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=59636=64911 BRANCH add_list_processes_with_open_files (branched from master) REVISION DETAIL

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added inline comments. INLINE COMMENTS > meven wrote in klistopenfiles.cpp:39 > The qAsConst wrapping is missing it seems. Yeah, I have only done this change locally ;) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson,

D23214: Add a BlockinApp util class to find process blockng access

hallas added a comment. In D23214#513408 , @meven wrote: > In D23214#513396 , @hallas wrote: > > > Hey @meven I have been working on the same thing D21760 -

D23214: Add a BlockinApp util class to find process blockng access

hallas added a comment. Hey @meven I have been working on the same thing D21760 - maybe we should consolidate our efforts? The code you have written looks very similar to what I have been doing :) As you can read in the review comments for D21760

D23205: [KProcessList] Optimize KProcessList::processInfo

hallas created this revision. hallas added reviewers: davidedmundson, broulik. Herald added a project: Frameworks. hallas requested review of this revision. REVISION SUMMARY Optimize KProcessList::processInfo on unix so that it doesn't iterate over all processes and then filter the list to

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas marked 3 inline comments as done. hallas added inline comments. INLINE COMMENTS > dfaure wrote in klistopenfiles.cpp:49 > This reminds me of KCoreAddons' new KProcessInfo class, although that one > doesn't actually do this. > > I see that FreeBSD and OSX have lsof (and the man page

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added a subscriber: cfeck. hallas added a comment. In D21760#493817 , @cfeck wrote: > You wrote "ported from Device Notifier". I haven't check in detail yet, but do other copyright holders need to be mentioned? I have added the

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added a comment. Ping :D REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas added a comment. First of, this is WIP, I just wanted to share this early to get some feedback. The reasoning for this is to generalize functionality for running `lsof`, this is currently in use by the Device Notifier applet and I would like to use it in Dolphin to fix bug #189302.

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

hallas created this revision. hallas added reviewers: davidedmundson, broulik. hallas added a project: Frameworks. hallas requested review of this revision. REVISION SUMMARY Add KListOpenFiles::listProcessesWithOpenFiles for retrieving the list of processes that has files open in the given

D21235: Add handling of fuseiso filesystem type

hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabdevice.cpp:88 > By default fuseiso seems to maintain also a file "~/.mtab.fuseiso", there may > be more information Yes it does :D After mounting an iso I have the following contents: /home/dha/test.iso /home/dha/mnt1

D21235: Add handling of fuseiso filesystem type

hallas added inline comments. INLINE COMMENTS > bruns wrote in fstabdevice.cpp:88 > @hallas: > cat /proc/mounts This is what I have in mounts: fuseiso on /home/dha/mnt1 type fuse.fuseiso (rw,nosuid,nodev,relatime,user_id=1000,group_id=1000) so there just isn't a whole lot of information -

D21235: Add handling of fuseiso filesystem type

hallas added inline comments. INLINE COMMENTS > broulik wrote in fstabdevice.cpp:88 > Can we maybe give it a better description then? For loop devices we show the > name of the backing file as description. Certainly :D I am also struggling a bit with how to best describe these FUSE mounted

D21235: Add handling of fuseiso filesystem type

hallas added inline comments. INLINE COMMENTS > fstabdevice.cpp:99 > +} else if (m_storageType == StorageType::Iso) { > +m_iconName = QLatin1String("media-optical-data"); > } else { I don't know if this is the best icon to use, but I think it should be something

D21235: Add handling of fuseiso filesystem type

hallas created this revision. hallas added reviewers: bruns, ngraham. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. hallas requested review of this revision. REVISION SUMMARY Add handling of fuseiso filesystem type. When a fuseiso filesystem is mounted

D20007: Add GetProcessList for retrieving the list of currently active processes

hallas added a comment. Thanks for the review! Landing it now. REPOSITORY R244 KCoreAddons BRANCH adds_kprocesslist (branched from master) REVISION DETAIL https://phabricator.kde.org/D20007 To: hallas, davidedmundson, broulik Cc: vonreth, adridg, elvisangelaccio,

D20007: Add GetProcessList for retrieving the list of currently active processes

hallas closed this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D20007 To: hallas, davidedmundson, broulik Cc: vonreth, adridg, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D20007: Add GetProcessList for retrieving the list of currently active processes

hallas added a comment. @davidedmundson - ping ? REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D20007 To: hallas, davidedmundson, broulik Cc: vonreth, adridg, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D20938: Add Mounts Backend

hallas added a comment. This patch has been superseeded by the wotk @bruns has done in D20995 - so closing this one :D REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D20938 To: hallas, #frameworks, ngraham, elvisangelaccio,

D20938: Add Mounts Backend

hallas abandoned this revision. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D20938 To: hallas, #frameworks, ngraham, elvisangelaccio, broulik, bruns Cc: svuorela, nicolasfella, ivan, kde-frameworks-devel, michaelh, ngraham, bruns

  1   2   >