const QString& fileName

const QString& fileName

const QString& name

should this really be a slot?

You meant const QString fileName, right?

You meant const QString fileName, right?

Cannot make it const, it's modified later on.

No, but there were only slots so far (also unconnected).

Remove space

( !fileName.isEmpty()

I'd say it would be safer to have 2 separate ifs, even if we know that the order is left to right, if somebody changes/adds here something and the fileName.at(0) will be before the isEmpty() then there will be a crash.

if (!fileName.isEmpty()) {
       if (fileName.at(0) != QDir::separator()) {

AbstractFileFilter::FileType fileType

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 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.

ok.

Ok. We should clean up here a bit maybe later and make private functions. Thanks for the pointer.

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.

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.

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 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.

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

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.

Actually it was me putting the FIXME there.