rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed.
In D12328#252246 <https://phabricator.kde.org/D12328#252246>, @rkflx wrote: > In D12328#250028 <https://phabricator.kde.org/D12328#250028>, @anemeth wrote: > > > Rebase on D12321 <https://phabricator.kde.org/D12321> > > > While the dependency tree is now looking good, the rebase did not work out correctly. While it might look correct on your local branch, on Phabricator you can clearly see that in this Diff the changes from the parent revision are visible, which should not be there. Please make sure to always check what ends up on Phabricator, as that's what your reviewers are seeing. > > `arc diff` will upload the commit range between `HEAD` and the upstream tracking branch. Here the upstream branch apparently is `master`, while it should be your local branch where D12321 <https://phabricator.kde.org/D12321> is living (you have a separate branch per Diff as written in the wiki, right?). You may want to look at my advice in D12321#249646 <https://phabricator.kde.org/D12321#249646> again. @anemeth Ping. I won't be able to help you with `kconf_update` or review the patch unless Phab shows me the proper patch! INLINE COMMENTS > kdiroperator.cpp:2165 > if (d->inlinePreviewState == Private::NotForced) { > - d->showPreviews = configGroup.readEntry(QStringLiteral("Previews"), > false); > + d->showPreviews = configGroup.readEntry(QStringLiteral("Preview > Thumbnails"), true); > + d->showPreviewsEnabledBeforeZoom = d->showPreviews; There's still some confusion in the wording wrt. `Show Preview`. How about naming your setting `Show Inline Previews` (inspired by `_k_toggleInlinePreview()`)? (The other option could be renamed to `Show Aside Preview` in a followup patch.) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns
