meven abandoned this revision.
meven marked 5 inline comments as done.
meven added a comment.
Moved to https://invent.kde.org/frameworks/kio/-/merge_requests/174
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D26407
To: meven, #frameworks, ngraham, broulik, dfaure
Cc: cf
meven added inline comments.
INLINE COMMENTS
> kmountpoint.cpp:444
> +// for /dir/link/dir/test will return result for
> /destLink/dir/test
> +return findByPath(fileinfo.symLinkTarget() +
> path.mid(cursor));
> +}
Need to pass the flag here.
REPOSIT
meven planned changes to this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D26407
To: meven, #frameworks, ngraham, broulik, dfaure
Cc: cfeck, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, GB_2,
michaelh, ngraham, bruns
cfeck added a comment.
Are more changes planned?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D26407
To: meven, #frameworks, ngraham, broulik, dfaure
Cc: cfeck, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, GB_2,
michaelh, ngraham, bruns
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
The approach makes sense to me.
INLINE COMMENTS
> kmountpoint.cpp:420
> +#else
> +const QString realname = path;
> #endif
You should rename this variable, it's no longer the
meven added a comment.
We might want to use Solid instead since it is capable of sending signals
when mnttab is updated, rather than like here having a mount point list updated
every 5 seconds.
With this code we might have issues with potential run condition : when there
is a mount within
meven updated this revision to Diff 75216.
meven added a comment.
Add a enum parameter to findByPath to choose its behavior regarding returning
first slow path
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26407?vs=73836&id=75216
BRANCH
arcpatch-D26407
RE
meven added a dependent revision: D26916: Allow to display in the kicker
appliation context menu the recently accessed directories.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D26407
To: meven, #frameworks, ngraham, broulik, dfaure
Cc: anthonyfieroni, kde-frameworks-deve
meven added a dependent revision: D26915: Allow to display in the task context
menu the recently accessed directories.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D26407
To: meven, #frameworks, ngraham, broulik, dfaure
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n,
meven planned changes to this revision.
meven added a comment.
> Really? that's not what all callers might want.
> e.g. for \"free disk space\" calculation we want the real final mountpoint
for that path.
> If you need something else, it should be a different method, or indeed a
flag
meven updated this revision to Diff 73836.
meven marked 3 inline comments as done.
meven added a comment.
Fix comment issues and improve syntax used
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26407?vs=73831&id=73836
BRANCH
arcpatch-D26407
REVISION DETAI
dfaure added inline comments.
INLINE COMMENTS
> kmountpoint.cpp:436
> +if (result && !result->probablySlow()) {
> +// check recursively its eventual parent dir transitive symlinks
> +int cursor = path.lastIndexOf(QLatin1Char('/'));
is the word "for" missing in this sentence?
meven marked an inline comment as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D26407
To: meven, #frameworks, ngraham, broulik, dfaure
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham,
bruns
meven added inline comments.
INLINE COMMENTS
> dfaure wrote in kmountpoint.cpp:443
> Why reuse and assign, compared to just `const QFileInfo fileinfo(parentPath)`?
>
> (Same for parentPath -- I prefer C++ over C)
I expected reviewers to tell to have declaration outside of loops...
> dfaure wro
meven updated this revision to Diff 73831.
meven marked 3 inline comments as done.
meven added a comment.
Use mid/lastIndexOf instead of split/join, use const assign in loop
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26407?vs=73819&id=73831
BRANCH
arcpat
dfaure added inline comments.
INLINE COMMENTS
> kmountpoint.cpp:443
> +parentPath = splitted.join(QDir::separator());
> +fileinfo = QFileInfo(parentPath);
> +if (fileinfo.isSymLink()) {
Why reuse and assign, compared to just `const QFileInfo fileinfo(parentPat
meven added inline comments.
INLINE COMMENTS
> dfaure wrote in kmountpoint.cpp:438
> Hmm, what if the symlink *is* the very last component, like your previous
> iteration tried to handle?
I made the incorrect assumption, I had already checked it
> dfaure wrote in kmountpoint.cpp:445
> I don't
meven updated this revision to Diff 73819.
meven marked 3 inline comments as done.
meven added a comment.
Make the recursive canonical work
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26407?vs=73690&id=73819
BRANCH
arcpatch-D26407
REVISION DETAIL
https
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kmountpoint.cpp:438
> +QStringList splitted = path.split(QDir::separator());
> +splitted.pop_back();
> +QString parentPath;
Hmm, what
meven updated this revision to Diff 73690.
meven added a comment.
Fix poping .length() > 0 ordering
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26407?vs=73687&id=73690
BRANCH
arcpatch-D26407
REVISION DETAIL
https://phabricator.kde.org/D26407
AFFECTED
meven updated this revision to Diff 73687.
meven marked an inline comment as done.
meven added a comment.
Check parent dir transitive symlinks, aka manual canonicalPath
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26407?vs=73524&id=73687
BRANCH
arcpatch-D2
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> meven wrote in kfileitem.cpp:787
> I meant this and this what KMountPoint does, better be precise here.
I see. Sorry, I hadn't realized that there is a Q_OS_A
meven added a comment.
I am thinking about adding some api to check status of nfs, sftp or smb
mounts by checking their hostname network reachability, to complement isSlow(),
and perhaps then running a stat of the mount with a timeout.
This would be kept in a cache with a short duration.
meven updated this revision to Diff 73524.
meven marked 4 inline comments as done.
meven added a comment.
Address review: Make KMountPoint::Ptr KMountPoint::List::findByPath
non-blocking, checking recursively intermediate symlinks
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://pha
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> meven wrote in kfileitem.cpp:787
> I did not mean to avoid android, just surfaced the comment in
> KMountPoint::currentMountPoints regarding its limitation to
meven updated this revision to Diff 73282.
meven marked 2 inline comments as done.
meven added a comment.
Move static variables to KFileItemPrivate::getMountPoints
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26407?vs=73261&id=73282
BRANCH
arcpatch-D26407
meven retitled this revision from "KFileItem: Improve isSlow to not block when
a network mount is unresponsive, make SkipMimeTypeFromContent skip only on
network fs" to "KFileItem: Improve isSlow to not block when a network mount is
unresponsive, make SkipMimeTypeFromContent skip only on slow fs
27 matches
Mail list logo
