This revision was automatically updated to reflect the committed changes.
Closed by commit R241:3042e12296db: KIO::PreviewJob::defaultPlugins() function
(authored by ngraham).
CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D8347?vs=21004=21005#toc
REPOSITORY
R241 KIO
CHANGES SINCE
ngraham updated this revision to Diff 21004.
ngraham added a comment.
Correcting the @since version number
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8347?vs=20975=21004
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8347
AFFECTED FILES
broulik added a comment.
5.39 was just released so it will be 5.40
REPOSITORY
R241 KIO
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin, markg
Cc: markg, anthonyfieroni, elvisangelaccio, #frameworks
ngraham marked 2 inline comments as done.
REPOSITORY
R241 KIO
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin, markg
Cc: markg, anthonyfieroni, elvisangelaccio, #frameworks
ngraham marked an inline comment as done.
ngraham added inline comments.
INLINE COMMENTS
> markg wrote in previewjob.h:198
> You miss an @since line.
I've updated the header documentation with @returns and @since, as well as
updated the documentation for the above availablePlugins() to help
ngraham updated this revision to Diff 20975.
ngraham added a comment.
Updating header documentation with @returns and @since
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8347?vs=20927=20975
BRANCH
master
REVISION DETAIL
ngraham added a comment.
I am indeed up for lots of bug reports. :) We can't find the bugs if many
(most?) users don't use the feature because it's off by default!
FWIW, I've always turned all plugins on as one of the first things I do on a
new install, and I have yet to see a
markg accepted this revision.
markg added a comment.
This revision is now accepted and ready to land.
In https://phabricator.kde.org/D8347#156930, @ngraham wrote:
> I still need to add a @since in the header documentation, after which point
hopefully @markg will approve. :)
+1
ngraham added a comment.
I still need to add a @since in the header documentation, after which point
hopefully @markg will approve. :)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin, markg
Cc: markg, anthonyfieroni,
broulik added a comment.
Agreed. I'd say if nobody objects this week this is good to go
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin, markg
Cc: markg, anthonyfieroni, elvisangelaccio, #frameworks
ngraham marked 2 inline comments as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin, markg
Cc: markg, anthonyfieroni, elvisangelaccio, #frameworks
ngraham marked an inline comment as done.
ngraham added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in previewjob.cpp:740
> const QStringList blackList = { QStringLiteral("textthumbnail") };
I wrote it the way I did on purpose, to facilitate people adding new entries if
and when
ngraham added a comment.
Right, I deliberately made this a change of the default values. I didn't want
to override existing user preferences.
I do agree that there should probably be a mechanism to immediately enable
new plugins as they're installed, but IMHO that's a different change
broulik added a comment.
Unfortunately this will not automatically enable newly installed thumbnailers
once the user has changed the setting, right? Dolphin does `readEntry("foo",
defaultPlugins)`, as soon as it's there new ones aren't taken into account.
Perhaps we need another function
markg requested changes to this revision.
markg added a comment.
This revision now requires changes to proceed.
I'm a bit skeptical about this function...
It in fact is all plugins minus (in this case) the text thumbnail with no way
to configure it afterwards.
That means a updates
anthonyfieroni added inline comments.
INLINE COMMENTS
> previewjob.cpp:740
> +{
> +QStringList blacklist = QStringList()
> +<< QStringLiteral("textthumbnail");
const QStringList blackList = { QStringLiteral("textthumbnail") };
REPOSITORY
R241 KIO
REVISION
ngraham edited the summary of this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin
Cc: elvisangelaccio, #frameworks
ngraham edited the summary of this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin
Cc: elvisangelaccio, #frameworks
ngraham added a dependent revision: D8352: Use
KIO::PreviewJob::defaultPlugins().
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin
Cc: elvisangelaccio, #frameworks
ngraham marked an inline comment as done.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin
Cc: elvisangelaccio, #frameworks
ngraham updated this revision to Diff 20927.
ngraham added a comment.
Whitespace fix
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8347?vs=20926=20927
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8347
AFFECTED FILES
ngraham updated this revision to Diff 20926.
ngraham marked an inline comment as done.
ngraham added a comment.
Style improvements/corrections
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8347?vs=20923=20926
BRANCH
master
REVISION DETAIL
ngraham marked 3 inline comments as done.
ngraham added inline comments.
INLINE COMMENTS
> elvisangelaccio wrote in previewjob.cpp:740-741
> Shouldn't the blacklist be user-configurable?
In fact, the whole list of plugins to enable is user-configurable in Dolphin.
The blacklist just defines
elvisangelaccio added inline comments.
INLINE COMMENTS
> previewjob.cpp:740-741
> +{
> +QStringList blacklist = QStringList()
> +<< QStringLiteral("textthumbnail");
> +
Shouldn't the blacklist be user-configurable?
> previewjob.cpp:745
> +foreach (const
ngraham updated this revision to Diff 20923.
ngraham added a comment.
Updating comments/documentation
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8347?vs=20916=20923
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8347
AFFECTED FILES
ngraham added a dependent revision: D7440: Turn on Dolphin icon previews by
default.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin
Cc: #frameworks
ngraham added reviewers: Frameworks, broulik, Dolphin.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8347
To: ngraham, #frameworks, broulik, #dolphin
Cc: #frameworks
ngraham created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
REVISION SUMMARY
Adds a new KIO::PreviewJob::defaultPlugins() function that returns all
available plugins minus ones specified in an internal blacklist.
28 matches
Mail list logo
