In D13018#266694, @bruns wrote:

Shouldn't this be handled by:
http://doc.qt.io/qt-5/qstandardpaths.html#setTestModeEnabled

In D13018#266626, @ngraham wrote:

I know you didn't change that, but it might be a nice enhancement (hint hint). :)

Run tests in an empty home dir

In D13018#266623, @ngraham wrote:

Shouldn't that be a part of the test itself? I don't think it's reasonable to require that testers and testing platforms manually run that every time.

as maximilianocuria said, before run the tests you need to create a temporary home path:

ok it should work now.

Fixed code style

Fixed all tests

Add missing url

Renamed variable

Updated @since tag

Use QString instead of char[]

Fixed typo

Fixed comments and code style

Fixed typo

Edit item fixed on: D9985 (do we have a bug report for that? or should I create one?)

Rever unrelated changes

Solve conflics with mainline

yes. I can confirm that entries recently created can not be edited , But if you create it re-open dolphin you can edit the entries. (Working on a fix right now).

yes this code should be skipped on the mainline, but was not.

looks good

Avoid crash while filtering urls

In D9347#185062, @elvisangelaccio wrote:

Yes, please merge on master and then rebase D9333.

In D9347#185062, @elvisangelaccio wrote:

Yes, please merge on master and then rebase D9333.

ok I will do that.

Fixed typo

In D9347#184887, @ngraham wrote:

Don't forget to merge your commits on the 17.12 branch back to master.

Removed TODO comments

In D9347#184697, @cfeck wrote:

Does it only avoid duplicate timeline:/ and search:/ ? I have duplicate 'Devices' and 'Removable Devices' sections since recently, and I cannot right-click to remove them.

In D9347#182655, @elvisangelaccio wrote:

What's the difference with D9333?

Check for hidden item in 'addItemFromSourceModel'

Check KIO version before use new API

Use explicit static function

Removed use of qWarning

Updated changes

In D8332#179633, @rkflx wrote:

This change caused a little bit of fallout for Gwenview. Apparently the review focussed more on the code, but less so on the behaviour in users of the class. I'm not complaining, but given one of our focus goals is on usability and quality of the basic apps, it would be great if:

• changes were tested more broadly in the future in addition to only looking at the code
• there was some help to fix the fallout
  
  Please head over to https://bugs.kde.org/show_bug.cgi?id=387824 if you can help Gwenview, thanks!
  
  ---
  
  In addition to Gwenview I also looked on lxr and did some testing based on what I found:
• The sidebar in KDirSelectDialog is now awful to use, because
  • The devices entry (which for some users is more important/useful/frequently used than the search entries) is hidden from view (bad) and requires scrolling (annoying). → We should discuss reordering or (even better) adding collapsing and then collapsing some groups by default.
  • The additional scrollbar makes the sidebar so small that you can't read most of the entries. → Add splitter and improve default width.
• To a lesser extent, this also applies to the normal file dialog (no scrollbar by default would be nice).
• Filesystem sidebars in KDevelop, #Okteta, Kile, Kate and Krusader: Some of the entries do not make sense in some of those apps at all, e.g. Videos/Images/…. Also some entries show an error message or are broken / show nothing at all (while the same entry works fine in Dolphin).
• That's it at first sight, luckily ;)
  
  Would be nice to fix those too… Let me know what's the best way forward here, i.e. what are Frameworks issues and where we'd need bugs filed against individual apps.

Added kf6 comments

Added documention for the new constructor

Fixed typo

Fixed typo

Avoid ABI break

@elvisangelaccio could you try again?

In D9242#177648, @elvisangelaccio wrote:

I can't apply this patch, can you please rebase it?

In D9242#177724, @elvisangelaccio wrote:

In D9242#177723, @renatoo wrote:

In D9242#177720, @ngraham wrote:

It appears we still need to land D8855 first.

I do not think this is the problem since that after run: arc patch D9242. I got both changes on my tree.

I get:

$ arc patch D9242
 INFO  Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D9242.
 INFO  Base commit is not in local repository; trying to fetch.
Checking patch src/filewidgets/kfileplacesmodel.h...
error: src/filewidgets/kfileplacesmodel.h: does not exist in index
Checking patch src/filewidgets/kfileplacesmodel.cpp...
error: src/filewidgets/kfileplacesmodel.cpp: does not exist in index
Checking patch autotests/kfileplacesmodeltest.cpp...
error: autotests/kfileplacesmodeltest.cpp: does not exist in index

 Patch Failed! 
Usage Exception: Unable to apply patch!

In D9242#177720, @ngraham wrote:

It appears we still need to land D8855 first.

In D9242#177703, @elvisangelaccio wrote:

In D9242#177654, @renatoo wrote:

In D9242#177648, @elvisangelaccio wrote:

I can't apply this patch, can you please rebase it?

done

arc patch D9242 still doesn't work, looks like something else is missing.

In D9242#177648, @elvisangelaccio wrote:

I can't apply this patch, can you please rebase it?

Rebased with master

Rebase with master

Removed override in favor of override

@elvisangelaccio ist that ok now?

Refactored unit test

Used new KFilePlacesModel::GroupHiddenRole to retrieve information instead of call the model method

used static_cast<int>() for cast enums

Avoid crash while hiding a item.

Looks good and work as expected

Looks good and works as expected

More fixes about rowCount usage

Used QVector insted of QList for m_indexMap

Avoid call model.rowCount() several times while on loop.