D14523: [import] Update file content overview after file type change

[Christoph Roick]

croick marked an inline comment as done.
croick added inline comments.

INLINE COMMENTS

> asemke wrote in ImportFileWidget.cpp:612
> 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...

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.

> fkristof wrote in ImportFileWidget.cpp:1106
> AbstractFileFilter::FileType fileType

`fileTypeChanged()` gets the fileType as `int`. I would have to explicitly cast this to `AbstractFileFilter::FileType` for the call of `updateContent()`. I thought I should stick to the current code, but I can change it, if you think that it is important.

> asemke wrote in ImportFileWidget.cpp:624
> 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.

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 (`fileName` in `absolutePath()` or just the temporary return value)  would be deleted once the function returns.

REPOSITORY
  R262 LabPlot

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

To: croick, #labplot
Cc: fkristof, asemke, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180801/bbbbb0d6/attachment-0001.html>