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

[Alexander Semke]

asemke added a subscriber: sgerlach.
asemke added inline comments.

INLINE COMMENTS

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

Let's decouple this problem from your patch that is fixing another problem. Let's finalize your patch and submitt it. Maybe @sgerlach can have another look at this piece of code where already another FIXME is placed.

> croick wrote in ImportFileWidget.cpp:1106
> `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.

Let's cast this to the enum value like we do this in ImportFileWidget::fileNameChanged() already and in all other widgets where we cast from integers coming from the comboboxes to the internal enum values, also in order to benefit from the compiler checks for the missing switch-cases. The current code in fileTypeChanged() is not good in this respect, yes. So, if it's not a big issue for you, let's clean up here in your patch.

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

Ok. This is a valid point. I forgot that we are having here a new function.

REPOSITORY
  R262 LabPlot

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

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