Looks fine to me as well.

What's the difference with D9333?

In D9347#182655, @elvisangelaccio wrote:

What's the difference with D9333?

This version is for v17.12.0 that does not require API changes on KIO.

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#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.

Devices are handled different, could you check if that is related with this fix? D9441

Cannot test uncommitted patches right now (no development environment installed), but that may indeed fix the issue, because devices appear, even if I have places hidded. Then selecting "Show All Entries" shows the duplicate devices entries.

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

In D9347#184887, @ngraham wrote:

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

For master, I was expecting to land this: D9333 which is the proper fix but requires the new KIO version.

Oh okay, great.

In D9347#184889, @renatoo wrote:

In D9347#184887, @ngraham wrote:

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

For master, I was expecting to land this: D9333 which is the proper fix but requires the new KIO version.

While Nate missed the purpose of D9333, technically he is right since merging is still required, isn't it?

Eventually someone will merge Applications/17.12 to master for a different fix, I would imagine. Or are you proposing to change to a cherry-pick model?

I guess merging now and then rebasing the patch for master on top, i.e. reverting some parts and applying the proper code fix, would be the way to go.

Yes, please merge on master and then rebase D9333.

Btw I cannot reproduce the duplicated places with dolphin master + kio master...

In D9347#185062, @elvisangelaccio wrote:

Yes, please merge on master and then rebase D9333.

ok I will do that.

Btw I cannot reproduce the duplicated places with dolphin master + kio master...

yes this will not be duplicated because kio is filtering only entries that has "OnlyInApp" with exactly the same name as the running app. Dolphin uses a extra suffix for the entries for each panel. This branch D9332 will allow to use alternative application names (necessary for dolphin), and this branch D9333 will make sure to remove the old entries and do not duplicate it.

In D9347#185062, @elvisangelaccio wrote:

Yes, please merge on master and then rebase D9333.

Hi, I tried to merge this to master but that causes some regressions and unit tests started to fail because this will ignore baloo urls that came from NEW KIO and the tests expect that urls be part of the list. The only easy way that I see it to be merged in master is that we need make sure that D9333 will land together otherwise that will cause regressions. Is that a option? I am not sure if that is a good idea.

In D9347#187655, @renatoo wrote:

In D9347#185062, @elvisangelaccio wrote:

Yes, please merge on master and then rebase D9333.

Hi, I tried to merge this to master but that causes some regressions and unit tests started to fail because this will ignore baloo urls that came from NEW KIO and the tests expect that urls be part of the list. The only easy way that I see it to be merged in master is that we need make sure that D9333 will land together otherwise that will cause regressions. Is that a option? I am not sure if that is a good idea.

I see, then let's merge with git merge -s recursive -Xours Applications/17.12, which will ignore this patch.

As a result of all this places meddling I now have Dolphin crash immediately on startup, see https://bugs.kde.org/show_bug.cgi?id=389147
m_indexMap inserts indices out bounds (non consequetive) because it skips timeline and baloo URLs from what I can tell and then it blows up.

I went back to Dolphin 64d2fd29819fa46c293e8c726c7df2ff00b332b3 (before the merge) and KIO cd9856d14552809251d11a5d0fc88d63928a7bab master where everything works.

Sounds like @lbeltrame's merge commit (1a6b3c0a2bab) did not go as planned. See above for why both branches aren't really "compatible" currently.

This will probably need @renatoo to figure out the situation…

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

I will make sure that: D9333 fixes that

Well, I didn't expect that master and Applications had differing behaviors... Anyway, is there a short-term solution short of undoing the merge (which is hard to do)?

I reverted this change in master: this doesn't solve all the problems (places aren't keeping edits now) but at least solves the crash, which is far more critical.

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).

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