<table><tr><td style="">croick marked an inline comment as done.<br />croick 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/D14523">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/D14523#inline-76770">View Inline</a><span style="color: #4b4d51; font-weight: bold;">asemke</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ImportFileWidget.cpp:612</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This part Christoph copied simply from another place... This logic we have in many places in the code where we striclty rely on the fixed ordering of the evaluation which has also to be guaranteed by the compiler. Many people are used to this and I think this is ok...</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I agree with Alexander. What's actually bothering me here, is the assumption that a relative path is supposed to be relative to the home folder. Maybe there is some reason for this, but it seems weird at least.</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/D14523#inline-76708">View Inline</a><span style="color: #4b4d51; font-weight: bold;">fkristof</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ImportFileWidget.cpp:1106</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">AbstractFileFilter::FileType fileType</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">fileTypeChanged()</tt> gets the fileType as <tt style="background: #ebebeb; font-size: 13px;">int</tt>. I would have to explicitly cast this to <tt style="background: #ebebeb; font-size: 13px;">AbstractFileFilter::FileType</tt> for the call of <tt style="background: #ebebeb; font-size: 13px;">updateContent()</tt>. I thought I should stick to the current code, but I can change it, if you think that it is important.</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/D14523#inline-76768">View Inline</a><span style="color: #4b4d51; font-weight: bold;">asemke</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ImportFileWidget.cpp:624</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I meant const reference. Though the difference with respect to the performance between copying an object and creating a reference to it might be very small for implicitly shared Qt classes (QString is implicitely shared), it's still matter of convention and of code styly. With const QString& you _explicitly_ say I do not want to copy and I'm not going to change the reference - this is what we try to consistenly use throughout the code.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I agree that arguments should be passed by reference, not by value, but I cannot have a const reference to a temporary object. The object it is referring to (<tt style="background: #ebebeb; font-size: 13px;">fileName</tt> in <tt style="background: #ebebeb; font-size: 13px;">absolutePath()</tt> or just the temporary return value)  would be deleted once the function returns.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R262 LabPlot</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D14523">https://phabricator.kde.org/D14523</a></div></div><br /><div><strong>To: </strong>croick, LabPlot<br /><strong>Cc: </strong>fkristof, asemke, kde-edu, narvaez, apol<br /></div>