D12328: Enable preview by default in the filepicker dialog

This revision was automatically updated to reflect the committed changes. Closed by commit R241:bec7f9d6a1ef: Enable preview by default in the filepicker dialog (authored by anemeth). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12328?vs=33167=33717 REVISION

D12328: Enable preview by default in the filepicker dialog

ngraham added a dependent revision: D12647: Move the inline preview button into the menu. BRANCH preview_default (branched from master) REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx, ngraham Cc: elvisangelaccio, abetts, rkflx, ngraham,

D12328: Enable preview by default in the filepicker dialog

ngraham accepted this revision. ngraham added a comment. The idea behind D12321 was to increase buy-in, since one of the objections was that most previews are a useless regression at small icon sizes. Text files still won't be previewed by default at any

D12328: Enable preview by default in the filepicker dialog

rkflx accepted this revision. rkflx added a subscriber: elvisangelaccio. rkflx added a comment. This revision is now accepted and ready to land. LGTM now (even though you ignored the "newline" I suggested…) I'd appreciate a second opinion from #Frameworks

D12328: Enable preview by default in the filepicker dialog

anemeth marked an inline comment as done. REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

anemeth updated this revision to Diff 33167. anemeth added a comment. - Rename .upd file - Add Version=5 to .upd file CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12328?vs=33103=33167 BRANCH preview_default (branched from master) REVISION DETAIL

D12328: Enable preview by default in the filepicker dialog

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. Thanks, `arc patch` now works great for me, automatically applying the dependent revision as it should. In D12328#254098 , @anemeth

D12328: Enable preview by default in the filepicker dialog

anemeth added a comment. The .upd is being automatically copied to `/usr/share/apps/kconf_update/` In the kconf_update log file `/home/alex/.kde4/share/apps/kconf_update/log/update.log` I found the following entry: 2018-04-25T21:41:56 Checking update-file

D12328: Enable preview by default in the filepicker dialog

anemeth updated this revision to Diff 33103. anemeth added a comment. - Actually add kconf_update files CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12328?vs=33102=33103 BRANCH preview_default (branched from master) REVISION DETAIL https://phabricator.kde.org/D12328

D12328: Enable preview by default in the filepicker dialog

anemeth updated this revision to Diff 33102. anemeth added a comment. - Merge branch 'conditional_preview' into preview_default - Added kconf_update CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12328?vs=33099=33102 BRANCH preview_default (branched from master) REVISION

D12328: Enable preview by default in the filepicker dialog

anemeth marked an inline comment as done. REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

anemeth marked an inline comment as not done. REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

rkflx added a comment. Thanks for the update, but please make sure not to lose the `kconf_update` changes… INLINE COMMENTS > kdiroperator.cpp:2163 > +d->showPreviews = configGroup.readEntry(QStringLiteral("Show Inline > Previews"), true); > +d->showPreviewsConfigEntry =

D12328: Enable preview by default in the filepicker dialog

anemeth marked an inline comment as done. REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

anemeth updated this revision to Diff 33099. anemeth added a comment. Rebase on D12321 CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12328?vs=32611=33099 BRANCH preview_default (branched from master) REVISION DETAIL

D12328: Enable preview by default in the filepicker dialog

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. In D12328#252246 , @rkflx wrote: > In D12328#250028 , @anemeth wrote: > > > Rebase on

D12328: Enable preview by default in the filepicker dialog

rkflx added a comment. In D12328#250028 , @anemeth wrote: > Rebase on D12321 While the dependency tree is now looking good, the rebase did not work out correctly. While it might look correct on your

D12328: Enable preview by default in the filepicker dialog

abetts added a comment. +1 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

ngraham added a comment. Hmm, I'm not sure the kconf update script is working. I just tried out the patch in its current state, and got this: Before: dev@dev-pc:~/repos/kio$ grep Preview ~/.config/kdeglobals Preview Width=269 Previews=true Show Preview=false

D12328: Enable preview by default in the filepicker dialog

anemeth updated this revision to Diff 32611. anemeth added a comment. - Add kconf_update files REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12328?vs=32603=32611 REVISION DETAIL https://phabricator.kde.org/D12328 AFFECTED FILES CMakeLists.txt

D12328: Enable preview by default in the filepicker dialog

rkflx added a comment. In D12328#250057 , @anemeth wrote: > In D12328#249648 , @rkflx wrote: > > > Could not run this yet, but IIRC deleting a key does not need a separate script, you should be able

D12328: Enable preview by default in the filepicker dialog

anemeth added a comment. In D12328#249648 , @rkflx wrote: > Could not run this yet, but IIRC deleting a key does not need a separate script, you should be able to do it inline in the `upd` with `RemoveKey`. how about this then:

D12328: Enable preview by default in the filepicker dialog

anemeth updated this revision to Diff 32603. anemeth added a comment. Rebase on D12321 REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12328?vs=32512=32603 REVISION DETAIL https://phabricator.kde.org/D12328 AFFECTED

D12328: Enable preview by default in the filepicker dialog

rkflx added a comment. In D12328#249641 , @anemeth wrote: > In D12328#249371 , @rkflx wrote: > > > However, the old key should not stay around in the config file, it should be removed. We have

D12328: Enable preview by default in the filepicker dialog

anemeth added a comment. In D12328#249371 , @rkflx wrote: > However, the old key should not stay around in the config file, it should be removed. We have `kconf_update` for that.

D12328: Enable preview by default in the filepicker dialog

ngraham added a comment. This depends on D12321 and enables D12326 ; there are a lot of changes in flight here. But the overall goal here is explained in T8552 : we want to make Short

D12328: Enable preview by default in the filepicker dialog

abetts added a comment. What would it look like if it was enabled by default? Screenshot? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg, rkflx Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. > This also enables it for users who previously disabled it. Hm, normally changing a default is allowed, while overriding custom user settings is discouraged. However, for a

D12328: Enable preview by default in the filepicker dialog

ngraham added a task: T8552: Polish Open/Save dialogs. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg Cc: ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

ngraham edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg Cc: ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

ngraham added a dependent revision: D12326: [WIP/open dependencies] In Short View, display icons on top and increase icon size. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12328 To: anemeth, #frameworks, #vdg Cc: ngraham, #frameworks, michaelh, bruns

D12328: Enable preview by default in the filepicker dialog

anemeth edited the summary of this revision. anemeth edited the test plan for this revision. anemeth added reviewers: Frameworks, VDG. anemeth added a subscriber: ngraham. anemeth added a dependency: D12321: Hide file preview when icon is too small. REPOSITORY R241 KIO REVISION DETAIL

D12328: Enable preview by default in the filepicker dialog

anemeth created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. anemeth requested review of this revision. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D12328 AFFECTED FILES