Use QStringLiteral or QLatin1String for normal strings
Details
Diff Detail
- Repository
- R244 KCoreAddons
- Branch
- complieWithoutRemoveDef
- Lint
No Linters Available - Unit
No Unit Test Coverage
Could you paste you compile error here please.
autotests/CMakeLists.txt | ||
---|---|---|
0–1 | Better to remove it. |
/home/veluri/ruqola_dep/kcoreaddons/autotests/kaboutdataapplicationdatatest.cpp:72: error: request for member ‘setOrganizationDomain’ in ‘aboutData2’, which is of non-class type ‘KAboutData(QLatin1String, QLatin1String, QLatin1String)’
aboutData2.setOrganizationDomain(OrganizationDomain2); ^
/home/veluri/ruqola_dep/kcoreaddons/autotests/kaboutdataapplicationdatatest.cpp:73: error: request for member ‘setDesktopFileName’ in ‘aboutData2’, which is of non-class type ‘KAboutData(QLatin1String, QLatin1String, QLatin1String)’
aboutData2.setDesktopFileName(QLatin1String(DesktopFileName2)); ^
/home/veluri/ruqola_dep/kcoreaddons/autotests/kaboutdataapplicationdatatest.cpp:75: error: no matching function for call to ‘KAboutData::setApplicationData(KAboutData (&)(QLatin1String, QLatin1String, QLatin1String))’
KAboutData::setApplicationData(aboutData2); ^
KAboutData aboutData2(QString::fromLatin1(AppName2), QString::fromLatin1(ProgramName2), QString::fromLatin1(Version2));
use QString::fromLatin1 is will work
Arguiment from method use QString and not QLatin1String()
QString::fromLatin1 returns a QString
autotests/kaboutdatatest.cpp | ||
---|---|---|
60 | no we need to keep const char ...[] and use QString::fromLatin1(...) in method as it's already done previously | |
autotests/kautosavefiletest.cpp | ||
83 | QStringLiteral(...) | |
autotests/kformattest.cpp | ||
36–44 | QStringLiteral here and others. | |
308 | QStringLiteral here too | |
autotests/kpluginfactorytest.cpp | ||
34 | QStringLiteral(...) | |
autotests/kpluginmetadatatest.cpp | ||
49 | Same | |
207–208 | Same | |
autotests/kshelltest.cpp | ||
58–61 | QStringLiteral... | |
84 | you can change QString::fromLatin1(...) by QStringLiteral in all code that you modified |
Missing all patch... please see "https://community.kde.org/Infrastructure/Phabricator"
"Step 2: Updating your diff.
After you upload the code the reviewer will take a look and give you some comments. If you get a thumbs up, you can skip this step. But often you will get a review like "looks good, we can take it if you fix problems x, y, z." After you make your changes, you can add an extra commit to the git branch.
$ git add -u
$ git commit
$ arc diff
"
I'm getting errors in processtest!!
/home/veluri/Qt/5.9.3/gcc_64/include/QtCore/qstringbuilder.h:357: error: no matching function for call to ‘QConcatenable<char [16]>::appendTo(const char [16], QChar*&)’
QConcatenable<A>::appendTo(p.a, out); ^
same kind of errors at 69, 71, 73
autotests/kprocesstest.cpp | ||
---|---|---|
93 | ? |
You still missing all patch...
Please use arc diff so we are sure that all patch is updated.
patch created for all the changes from starting
This diff contains total changes in all files...
kcoreaddons compilation done!!
autotests/kprocesstest.cpp | ||
---|---|---|
93 | Yeah working, I changed it But, Unable to eliminate this compile error | |
autotests/kstringhandlertest.cpp | ||
45 | Here char i. e., (' ') is converted into string i.e., (" ") and then QLatin1String is applied. Since, we can't convert char into QLatin1String | |
76 | Here too zwsp was QChar, it is converted into QString. Same for below lines also!! |
Hi!!,
Code updated. Please let me know any changes are there yet to be done
Regards.
VM
The rest looks good.
autotests/kshelltest.cpp | ||
---|---|---|
139 ↗ | (On Diff #24493) | Does this test still pass? You can't use QStringLiteral when there are escape characters like \x21 in the literal. See https://commits.kde.org/clazy/49825f06fda23989961cfa41cd1576b58e2bcc5d for details. Please make sure to run ctest before and after. |
indeed code will be broken if we don't use QString::fromUtf8.
Could you fix it please ?
Fixed :)
StringEncodings doc helped me to understand encoding and decoding.
Still if there any modifications kindly let me know.
Thank You.
When I try to compile your patch I have this compile error:
Do you are sure that you rebuild all ?
/compile/kde5/framework/frameworks/kcoreaddons/autotests/kaboutdatatest.cpp: In member function ‘void KAboutDataTest::testLicenseSPDXID()’:
/compile/kde5/framework/frameworks/kcoreaddons/autotests/kaboutdatatest.cpp:330:91: error: ‘QString::QString(const char*)’ is private within this context
QLatin1String(ShortDescription), KAboutLicense::GPL_V2); ^
In file included from /opt/qt5.10/include/QtCore/QString:1:0,
from /compile/kde5/framework/frameworks/kcoreaddons/src/lib/kaboutdata.h:30, from /compile/kde5/framework/frameworks/kcoreaddons/autotests/kaboutdatatest.cpp:21:
/opt/qt5.10/include/QtCore/qstring.h:818:5: note: declared private here
QString(const char *ch); ^~~~~~~
/compile/kde5/framework/frameworks/kcoreaddons/autotests/kaboutdatatest.cpp:330:91: error: ‘QString::QString(const char*)’ is private within this context
QLatin1String(ShortDescription), KAboutLicense::GPL_V2); ^
In file included from /opt/qt5.10/include/QtCore/QString:1:0,
from /compile/kde5/framework/frameworks/kcoreaddons/src/lib/kaboutdata.h:30, from /compile/kde5/framework/frameworks/kcoreaddons/autotests/kaboutdatatest.cpp:21:
/opt/qt5.10/include/QtCore/qstring.h:818:5: note: declared private here
Yeah, I have rebuilt it !! NO ERRORS compiling well. I'm not getting any errors like that :/
I could see few warnings but not in kaboutdatatest.cpp
I'm compiling it in Qt 5.9.3 version.
Ok I understand why you don't have this problem you need to rebase patch with last kcoreaddons version.
So code was added.
Please rebase, make sure that it builds and upload patch.
Regards.
- use QLating1String
- compile commenting remove_definition(DQT_NO_CAST_FROM_ASCII
- Make necessary changes according to comments in D9420
- Use QString::fromUtf8(..) to decode an escape sequence like \x21
- Merge with origin/master
- Rewrite strings using QLatin1String() and QString::fromLatin1()
Sry for the late patch :))
I was busy with setting up Ruqola...
I have one doubt that why this(kdirwatch_unittest.cpp ) file is in autotests?? It should be in unit tests right?
Thank You for your patience.
You still forgot to launch autotest...
see:
"laurent@linux-5nvn:/compile/kde5/framework/frameworks/kcoreaddons/build> ./bin/kaboutdatatest
- Start testing of KAboutDataTest *****
Config: Using QtTest library 5.10.1, Qt 5.10.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 7.2.1 20171020 [gcc-7-branch revision 253932])
PASS : KAboutDataTest::initTestCase()
PASS : KAboutDataTest::testLongFormConstructorWithDefaults()
PASS : KAboutDataTest::testLongFormConstructor()
PASS : KAboutDataTest::testShortFormConstructor()
FAIL! : KAboutDataTest::testSetAddLicense() Compared values are not the same
Actual (aboutData.licenses().at(2).text()): "free to write, reading forbidden" Expected (QLatin1String(LicenseFileText)) : "free to write, reading forbidden, in the file" Loc: [/compile/kde5/framework/frameworks/kcoreaddons/autotests/kaboutdatatest.cpp(260)]
PASS : KAboutDataTest::testSetProgramIconName()
PASS : KAboutDataTest::testSetDesktopFileName()
PASS : KAboutDataTest::testCopying()
PASS : KAboutDataTest::testKAboutDataOrganizationDomain()
PASS : KAboutDataTest::testLicenseSPDXID()
PASS : KAboutDataTest::testLicenseOrLater()
PASS : KAboutDataTest::cleanupTestCase()
Totals: 11 passed, 1 failed, 0 skipped, 0 blacklisted, 3ms
"
> I found the porting error in line 260. Please compare you change in 260.
You must launch autotest to see if you created a bug or not.
You need to compare before and after your patch and see if you have some failed.
Note:
In kdirwatch_qfswatch_unittest KDirWatch_UnitTest::benchNotifyWatcher() fails before and after changes in kdirwatch_unittest.cpp file
This fails to build on Windows, please see https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20WindowsMSVCQt5.9/49/consoleText
Can you please look into and commit a fix for this as soon as possible?