D23538: [dolphin] Animate gifs on preview
Ismael Asensio
noreply at phabricator.kde.org
Fri Sep 13 18:08:31 BST 2019
iasensio added a comment.
In D23538#529632 <https://phabricator.kde.org/D23538#529632>, @ngraham wrote:
> Ugh, those kinds of if/else blocks are always confusing. I would (in another patch) re-arrange it so everything currently in the else block comes first (`if (!usePhonon) { blabla`) to get that small bit out of the way first so the more complex logic can come next without an else block after it.
Sure! This method is loudly calling for refactoring, and I think @meven has that also in mind.
As for this patch, I moved the lines to the `isPhonon:else` block as suggested, which in fact helps to clarify the structure.
For later work on this code, I'm looking forward to your refactoring which, I think, will simplify the code readibility a lot, and mainly reduce the bug-prone capability of the current one.
(i.e. if you look carefully, there can also be some funny issues when moving between non-baloo searches ('isSearchUrl') and phonon thumbnails)
REPOSITORY
R318 Dolphin
REVISION DETAIL
https://phabricator.kde.org/D23538
To: iasensio, #dolphin, #vdg, ngraham, elvisangelaccio
Cc: pino, fuksitter, meven, broulik, kfm-devel, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190913/a9f9b64f/attachment.htm>
More information about the kfm-devel
mailing list