D17245: Add string formatting function to property info

This revision was automatically updated to reflect the committed changes. Closed by commit R286:4bedfd6609c6: Add string formatting function to property info (authored by astippich). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=51271=51330

D17245: Add string formatting function to property info

bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH display_value REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel, #baloo,

D17245: Add string formatting function to property info

astippich updated this revision to Diff 51271. astippich added a comment. - rebase onto current master - rename header - update docs REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=48690=51271 BRANCH display_value REVISION DETAIL

D17245: Add string formatting function to property info

bruns added a comment. `src/formatstrings.h` should be named `src/formatstrings_p.h` REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh,

D17245: Add string formatting function to property info

astippich added a comment. Can someone please give this a yes or a no? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich,

D17245: Add string formatting function to property info

astippich added a comment. ping! REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, bruns, abrahams

D17245: Add string formatting function to property info

astippich added a comment. @broulik since you've made the feature request, do you mind giving this a quick look? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel,

D17245: Add string formatting function to property info

astippich updated this revision to Diff 48690. astippich marked an inline comment as done. astippich added a comment. - add _p to header define REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=47816=48690 BRANCH display_value REVISION

D17245: Add string formatting function to property info

bruns added a comment. I think this is ok now, but I would like to have one +1 from any other frameworks developer. INLINE COMMENTS > formatstrings.h:21 > +#ifndef KFILEMETADATA_FORMATSTRINGS_H > +#define KFILEMETADATA_FORMATSTRINGS_H > This is a private headers, and thus should have a

D17245: Add string formatting function to property info

astippich marked 3 inline comments as done. astippich added a comment. Thanks for the review! INLINE COMMENTS > bruns wrote in formatstrings.cpp:50 > Why is midnight an invalid time? Same with the previous discussing around the orientation values, I simply copied the code. My plan was to

D17245: Add string formatting function to property info

astippich updated this revision to Diff 47816. astippich added a comment. - remove static kformat - always show time - change default display function handling REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=47660=47816 BRANCH

D17245: Add string formatting function to property info

bruns added a reviewer: Frameworks. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien, #frameworks Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, bruns, abrahams

D17245: Add string formatting function to property info

bruns added inline comments. INLINE COMMENTS > formatstrings.cpp:27 > + > +const KFormat FormatStrings::form; > + I don't think using a static default constructed KFormat is a good idea ... > formatstrings.cpp:50 > +QTime time = dt.time(); > +if (!time.hour() && !time.minute()

D17245: Add string formatting function to property info

astippich added a comment. String freeze is approaching next week, and I would like to get this in for 5.54 so that I can start porting Dolphin and baloo-widgets. Anyone? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns,

D17245: Add string formatting function to property info

astippich updated this revision to Diff 47660. astippich added a comment. - improve code readability REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=46859=47660 BRANCH display_value REVISION DETAIL https://phabricator.kde.org/D17245

D17245: Add string formatting function to property info

astippich marked 3 inline comments as done. astippich added a comment. any further comments/objections? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D17245 To: astippich, broulik, bruns, mgallien Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov,

D17245: Add string formatting function to property info

astippich marked 3 inline comments as done. astippich added a comment. I'm wondering, anything else to do if I add a new dependency to KFileMetaData? INLINE COMMENTS > bruns wrote in propertyinfotest.cpp:63 > I think this should be "44.1 kHz". Insert the correct expected value and make > it

D17245: Add string formatting function to property info

astippich updated this revision to Diff 46859. astippich added a comment. - small fixes REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=46688=46859 BRANCH display_value REVISION DETAIL https://phabricator.kde.org/D17245 AFFECTED FILES

D17245: Add string formatting function to property info

bruns added inline comments. INLINE COMMENTS > propertyinfotest.cpp:63 > +PropertyInfo sampleRate(Property::SampleRate); > +QCOMPARE(sampleRate.formatAsDisplayString(QVariant(44100)), > QStringLiteral("44,1 kHz")); > +PropertyInfo bitRate(Property::BitRate); I think this should be

D17245: Add string formatting function to property info

astippich updated this revision to Diff 46688. astippich added a comment. - update strings for image orientation REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=46509=46688 BRANCH display_value REVISION DETAIL

D17245: Add string formatting function to property info

astippich added inline comments. INLINE COMMENTS > bruns wrote in formatstrings.cpp:83 > IMHO thats not better at all - what does "top, left" refer to? And whats the > difference to "left, top"? > > Every orientation save "normal" and "rotated 180" is ambiguous without > further description.

D17245: Add string formatting function to property info

bruns added inline comments. INLINE COMMENTS > astippich wrote in formatstrings.cpp:83 > How about using the descriptions from D15503 > , which are also consistent with digiKam > and Gwenview? The best solution is probably to make D15503 >

D17245: Add string formatting function to property info

astippich added inline comments. INLINE COMMENTS > bruns wrote in formatstrings.cpp:83 > Of course I can look this up. But this information is not available when the > user looks at the string. > > Probably we should ask what is the target audience here? The average user > only wants the

D17245: Add string formatting function to property info

bruns added inline comments. INLINE COMMENTS > astippich wrote in formatstrings.cpp:83 > CCW, see > https://docs.microsoft.com/en-us/uwp/api/windows.storage.fileproperties.photoorientation. Of course I can look this up. But this information is not available when the user looks at the string.

D17245: Add string formatting function to property info

astippich added inline comments. INLINE COMMENTS > bruns wrote in formatstrings.cpp:81 > Thats somewhat ambigous - flipped along the vertical axis, or top/bottom > flipped? Actually I don't know, I just copied the code from Dolphin into here. Seems to be a common term though according to

D17245: Add string formatting function to property info

bruns added inline comments. INLINE COMMENTS > formatstrings.cpp:81 > +case 3: string = i18nc("@item:intable image orientation", "180° > rotated"); break; > +case 4: string = i18nc("@item:intable image orientation", "Vertically > flipped"); break; > +case 5: string =

D17245: Add string formatting function to property info

ngraham added inline comments. INLINE COMMENTS > formatstrings.cpp:82 > +case 4: string = i18nc("@item:intable image orientation", "Vertically > flipped"); break; > +case 5: string = i18nc("@item:intable image orientation", "Transposed"); > break; > +case 6: string =

D17245: Add string formatting function to property info

astippich updated this revision to Diff 46509. astippich added a comment. - make KCoreAddons a required framework REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17245?vs=46508=46509 BRANCH display_value REVISION DETAIL

D17245: Add string formatting function to property info

astippich created this revision. astippich added reviewers: broulik, bruns, mgallien. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY Adds the ability to format the metadata value