D11245: infopanel: Add choice of date display formats

Mark Gaiser noreply at phabricator.kde.org
Wed Mar 14 22:18:18 GMT 2018


markg requested changes to this revision.
markg added a comment.
This revision now requires changes to proceed.


  In D11245#224414 <https://phabricator.kde.org/D11245#224414>, @ngraham wrote:
  
  > I think having it in the context menu makes enough sense; the toggle for the preview is already there, so there's already precedent for having the Information Panel's UI be controlled via context menu. +1.
  
  
  @broulik and I seem to think that it should be in the settings, not under the RMB.
  To be fair, i'm trusting your judgement so i'm fine either way, but i'm "leaning" towards in the settings as well.

INLINE COMMENTS

> informationpanelcontent.cpp:119
>      m_metaDataWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Minimum);
> -
> +    m_metaDataWidget->setDateFormat((Baloo::DateFormats) InformationPanelSettings::dateFormat());
> + 

No c-style cast.

> informationpanelcontent.cpp:198
>      if (m_metaDataWidget) {
> +        m_metaDataWidget->setDateFormat((Baloo::DateFormats) InformationPanelSettings::dateFormat());
>          m_metaDataWidget->show();

No c-style cast.

> informationpanelcontent.cpp:290
> +    dateformatAction->setCheckable(true);
> +    dateformatAction->setChecked(InformationPanelSettings::dateFormat() == (int) Baloo::DateFormats::ShortFormat);
>      popup.addSeparator();

No c-style cast.

> informationpanelcontent.cpp:308-311
> +        auto dateFormat = static_cast<int>(
> +            isChecked
> +            ? Baloo::DateFormats::ShortFormat
> +            : Baloo::DateFormats::LongFormat);

Err, auto for int.. No, just int! Don't misuse auto.
Also, put this on one line. Nobody formats a cast like that and it imho makes it less readable.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D11245

To: michaelh, #dolphin, markg
Cc: broulik, markg, ngraham, spoorun, navarromorales, isidorov, firef, andrebarros, alexeymin, genaxxx, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180314/baea54ea/attachment.htm>


More information about the kfm-devel mailing list