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