<table><tr><td style="">astippich added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D11365">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11365#inline-58607">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mgallien</span> wrote in <span style="color: #4b4d51; font-weight: bold;">taglibextractor.cpp:363</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Is it really needed to push directly a list ?<br />
You are ignoring the fact that the original developer intended add to be called once for each value. I would prefer to keep doing that until we can be absolutely sure that nothing breaks if we change that.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">That was actually the point for this. In propertyinfo, it says that this property is a stringlist, but it was actually never one, just a simple string. <br />
For multiple values, there will be multiple entries in the property map with a single string.  This is not considered within Elisa at least. We could also go the other way and adjust the properties to match the behavior. <br />
<a href="https://phabricator.kde.org/p/michaelh/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@michaelh</a> agreed that stringlists are easier to handle</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11365#inline-58612">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mgallien</span> wrote in <span style="color: #4b4d51; font-weight: bold;">taglibextractor.cpp:389</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Are you sure we need this cast ? I do not think we will ever need to have negative track numbers added. The original type is unsigned int and should exactly convey the fact that we do not expect negative numbers.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I changed it so it matches its associated value type, I could also change the type in propertyinfo to uint</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11365#inline-58611">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mgallien</span> wrote in <span style="color: #4b4d51; font-weight: bold;">taglibextractor.cpp:393</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">idem</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This one is a litte bit more tricky, as the year property could theoretically also be negative (B.C.). Not really relevant for music, but maybe for written documents, and still only very rarily :)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R286 KFileMetaData</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11365">https://phabricator.kde.org/D11365</a></div></div><br /><div><strong>To: </strong>astippich, Frameworks, Baloo, mgallien, michaelh<br /><strong>Cc: </strong>michaelh, Frameworks, ashaposhnikov, astippich, spoorun, nicolasfella, ngraham, alexeymin<br /></div>