make && ctest
Details
Details
- Reviewers
dfaure - Group Reviewers
Frameworks - Commits
- R241:54ccb1ac4883: More fixes to compile without implicit conversion from ASCII/ByteArray
Diff Detail
Diff Detail
- Repository
- R241 KIO
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Thanks!
| src/core/forwardingslavebase.cpp | ||
|---|---|---|
| 91 | QLatin1String is sufficient for comparisons (and cheaper) | |
| src/core/kremoteencoding.cpp | ||
| 95 | KF6 TODO: return QByteArray | |
| src/core/slavebase.h | ||
| 1021 | This method should be const | |
| src/ioslaves/ftp/ftp.cpp | ||
| 1220 | Should this be remoteEncoding()->decode(...) given that the method will then use q->remoteEncoding()->encode()? (I'm a bit confused with kremoteencoding, I could be wrong) | |
| src/kcms/kio/kcookiespolicies.cpp | ||
| 367 | fromLatin1 is enough for adviceToStr, like you did on line 177 (so it's inconsistent) (repeats 3 more times) | |
| src/kcms/kio/smbrodlg.cpp | ||
| 122 | You could remove the QChar() around I guess. | |
| src/kioexec/main.cpp | ||
| 199 | Ouch. QFileInfo info(it->path) should be enough. | |
| 262 | Same here | |
Comment Actions
Address review comments.
Rebase, and fix some more instances of casting from ASCII that crept up since the diff was created.
| src/core/slavebase.h | ||
|---|---|---|
| 1021 | A B C, nothing changed inside make it const, D E F. | |
| src/ioslaves/ftp/ftp.cpp | ||
| 1220 | Yes, that makes much more sense; using remoteEncoding()->decode() makes this bit of code consistent with the reset of how the ftp ioslave handles encoding. | |
| src/kcms/kio/kcookiespolicies.cpp | ||
| 367 | Apparently I was confused by the I18N_NOOP calls in adviceToStr, but looking again now, the translations aren't saved in the config file... | |
| src/kcms/kio/smbrodlg.cpp | ||
| 122 | You're just being polite. | |
| src/kioexec/main.cpp | ||
| 199 | In my defence, since that looks a bit too crazy even to me, I was worried about files with names that have encoding issues, e.g. the dreaded �; but looking up just a little at the code I see that fileList was written by the code, so if the encoding did go south, that ship has sailed and already has sunk :) | |
Comment Actions
Ping. (I want to get this committed to close the gates before any more implicit conversions sneak in :)).
| src/core/slavebase.h | ||
|---|---|---|
| 1021 | Yep ;) Hmm, also it's in the middle of member variables, better not mix them up. | |
| src/filewidgets/kfileplacesmodel.cpp | ||
| 277 | This is more commonly written as "static const" | |
| src/kioexec/main.cpp | ||
| 199 | In any case, encoding issues are for QString<->QByteArray conversions. Any time we can stick to QString<->QString like here, we're safe :) | |
| src/kpasswdserver/autotests/kpasswdservertest.cpp | ||
| 304 | [strange mix of QLatin1String and QStringLiteral, but harmless] | |
| src/filewidgets/kfileplacesmodel.cpp | ||
|---|---|---|
| 277 | This does not work. Our translation system cannot extract from something like I18NC_NOOP(fsBookmarks, QStringLiteral("Desktop")) all the strings below with I18NC_NOOP should be left as is. Thanks. | |
| src/filewidgets/kfileplacesmodel.cpp | ||
|---|---|---|
| 277 | Oh, right, indeed. Duplication FTW :-) | |
Comment Actions
- Keep I18NC_NOOP working
- QLatin1String where appropriate
- Rename mProtocolToStr() to protocolName(), and make it private (only used in slavebase)
| src/filewidgets/kfileplacesmodel.cpp | ||
|---|---|---|
| 277 | Fixed (and noted for the future). Thanks. | |
| src/filewidgets/kfileplacesmodel.cpp | ||
|---|---|---|
| 282 | The QStringLiteral here might create trouble for the translation extraction scripts? I think the proper fix is to change the signature of createSystemBookmark to take two QByteArray arguments. Or you could even keep const char* for the context (the first one of the two) since it's dropped, no point in creating a QByteArray out of it. | |
| src/kpasswdserver/autotests/kpasswdservertest.cpp | ||
| 325 | [same as above] | |
Comment Actions
Per dfaure's recommendation, change createSystemBookmark 2nd and 3rd params to take a const char * (since this is the translation context which will be dropped) and a QByteArray, respectively.