D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  I made a fix. I wait a rebuild on windows to see if it's ok

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: bcooksley, dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Ben Cooksley]

bcooksley added a comment.


  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?

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: bcooksley, dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:53f3dce3f012: Compile commenting 
remove_defintion(QT_NO_CAST_FROM_ASCII ) (authored by velurimithun).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=25712=25751

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kdirwatch_unittest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kprocesstest.cpp
  autotests/krandomtest.cpp
  autotests/kshelltest.cpp
  autotests/kstringhandlertest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent accepted this revision.
mlaurent added a comment.
This revision is now accepted and ready to land.


  Thanks

REPOSITORY
  R244 KCoreAddons

BRANCH
  complieWithoutRemoveDef

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun added a comment.


  **Note:**
  
  In kdirwatch_qfswatch_unittest  KDirWatch_UnitTest::benchNotifyWatcher() 
fails before and after changes in kdirwatch_unittest.cpp file

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 25712.
velurimithun added a comment.


  - Compare license file correctly

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=25677=25712

BRANCH
  complieWithoutRemoveDef

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kdirwatch_unittest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kprocesstest.cpp
  autotests/krandomtest.cpp
  autotests/kshelltest.cpp
  autotests/kstringhandlertest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent requested changes to this revision.
mlaurent added a comment.
This revision now requires changes to proceed.


  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.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun added a comment.


  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.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 25677.
velurimithun added a comment.


  - use QLating1String
  - compile commenting remove_definition(DQT_NO_CAST_FROM_ASCII
  - Make necessary changes according to comments in 
https://phabricator.kde.org/D9420
  - Use QString::fromUtf8(..) to decode an escape sequence like \x21
  - Merge with origin/master
  - Rewrite strings using QLatin1String() and QString::fromLatin1()

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=25076=25677

BRANCH
  complieWithoutRemoveDef

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kdirwatch_unittest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kprocesstest.cpp
  autotests/krandomtest.cpp
  autotests/kshelltest.cpp
  autotests/kstringhandlertest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  Ping bis ?:)

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  Ping ?

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  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.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun added a comment.


  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.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent requested changes to this revision.
mlaurent added a comment.
This revision now requires changes to proceed.


  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

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun marked an inline comment as done.
velurimithun added a comment.


  Fixed :)
  
  StringEncodings  doc helped 
me to understand encoding and decoding.
  
  Still if there any modifications kindly let me know.
  
  Thank You.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 25076.
velurimithun added a comment.


  - Use QString::fromUtf8(..) to decode an escape sequence like \x21

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24493=25076

BRANCH
  complieWithoutRemoveDef

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kprocesstest.cpp
  autotests/krandomtest.cpp
  autotests/kshelltest.cpp
  autotests/kstringhandlertest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  indeed code will be broken if we don't use QString::fromUtf8.
  Could you fix it please ?

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[David Faure]

dfaure added a comment.


  The rest looks good.

INLINE COMMENTS

> kshelltest.cpp:139
>  #else
> -QCOMPARE(sj("\"~qU4rK\" 'text' 'jo'\"jo\" $'crap' $'\\'\\e\\x21' 
> ha\\ lo \\a", KShell::NoOptions, ),
> - QString("'~qU4rK' text jojo crap '\\'\\''\x1b!' 'ha lo' a"));
> +QCOMPARE(sj(QStringLiteral("\"~qU4rK\" 'text' 'jo'\"jo\" $'crap' 
> $'\\'\\e\\x21' ha\\ lo \\a"), KShell::NoOptions, ),
> + QStringLiteral("'~qU4rK' text jojo crap '\\'\\''\x1b!' 'ha lo' 
> a"));

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.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: dfaure, #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun added a comment.


  Hi!!,
  
  Code updated. Please let me know any changes are there yet to be done
  
  Regards.
  VM

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun marked 4 inline comments as done.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 24493.
velurimithun added a comment.


  - Make necessary changes according to comments in 
https://phabricator.kde.org/D9420
  
  Use QLatin1Char for char

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24452=24493

BRANCH
  complieWithoutRemoveDef

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kprocesstest.cpp
  autotests/krandomtest.cpp
  autotests/kshelltest.cpp
  autotests/kstringhandlertest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added inline comments.

INLINE COMMENTS

> kprocesstest.cpp:47
>  p.setProcessChannelMode(QProcess::MergedChannels);
> -p.start(gargv[0], QStringList() << QString::number(how) << 
> QStringLiteral("--nocrashhandler"));
> +p.start(QString::fromLatin1(gargv[0]), QStringList() << 
> QString::number(how) << QString::fromLatin1("--nocrashhandler"));
>  p.waitForFinished();

nocrashhandler why do you changed as QString::fromLatin1 ? it's QStringList() 
so it can work no ?

> kshelltest.cpp:167
>  
> -const QString unicodeSpaceFileName = "test　テスト.txt"; // #345140
> +const QString unicodeSpaceFileName = QLatin1String("test　テスト.txt"); // 
> #345140
>  QCOMPARE(sj(unicodeSpaceFileName, KShell::AbortOnMeta | 
> KShell::TildeExpand, ),

QLAtin1String for japan char ?

> velurimithun wrote in 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

QLatin1Char(' ') ?

> kstringhandlertest.cpp:49
>  expected.clear();
> -expected << "Split" << "me" << "up ! I'm bored ! OK ?";
> -QCOMPARE(KStringHandler::perlSplit(QRegExp("[! ]"), "Split me up ! I'm 
> bored ! OK ?", 3), expected);
> +expected << QLatin1String("Split") << QLatin1String("me") << 
> QLatin1String("up ! I'm bored ! OK ?");
> +QCOMPARE(KStringHandler::perlSplit(QRegExp(QLatin1String("[! ]")),

QStringLiteral no ?

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun added inline comments.

INLINE COMMENTS

> mlaurent wrote in kprocesstest.cpp:93
> ?
> QStringLiteral works fine with QStringList

Yeah working!!, I changed it!!

But, Unable to eliminate this compile error

> kstringhandlertest.cpp:45
> +expected << QLatin1String("kparts") << QLatin1String("reaches") << 
> QLatin1String("the parts other parts can't");
> +QCOMPARE(KStringHandler::perlSplit(QLatin1String(" "),
> +   QLatin1String("kparts reaches the 
> parts other parts can't"), 3), expected);

Here char i. e., (' ') is converted into string i.e., (" ") and then 
QLatin1String is applied.

Since, we can't convert char into QLatin1String

> kstringhandlertest.cpp:76
>  QTest::newRow("underscores") << "foo_bar_baz"
> - << QString("foo_" + zwsp + "bar_" + zwsp + 
> "baz");
> + << QString(QStringLiteral("foo_") + 
> QString(zwsp) + QStringLiteral("bar_") + QString(zwsp) + 
> QStringLiteral("baz"));
>  

Here too **zwsp** was QChar, it is converted into QString.

Same for below lines also!!

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 24452.
velurimithun added a comment.


  patch created for all the changes from starting
  
  This diff contains total changes in all files...
  
  kcoreaddons compilation done!!

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24238=24452

BRANCH
  complieWithoutRemoveDef

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kprocesstest.cpp
  autotests/krandomtest.cpp
  autotests/kshelltest.cpp
  autotests/kstringhandlertest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  no news ?

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  You still missing all patch...
  Please use arc diff so we are sure that all patch is updated.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 24238.
velurimithun marked 2 inline comments as done.

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24199=24238

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

AFFECTED FILES
  autotests/kaboutdatatest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kprocesstest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added inline comments.

INLINE COMMENTS

> velurimithun wrote in kprocesstest.cpp:93
> These strings are going as arguments for QStringList() method that why I used 
> QString::FromLatin1()

?
QStringLiteral works fine with QStringList

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

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

INLINE COMMENTS

> mlaurent wrote in kaboutdatatest.cpp:184
> QStringLiteral doen't work here ?

Yeah working, I'll change it

> mlaurent wrote in kprocesstest.cpp:93
> QStringLiteral

These strings are going as arguments for QStringList() method that why I used 
QString::FromLatin1()

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added inline comments.

INLINE COMMENTS

> desktoptojsontest.cpp:84
>  QByteArray input =
> -// include an insignificant group
> -"[Some Group]\n"
> -"Foo=Bar\n"
> -"\n"
> -"[Desktop Entry]\n"
> -// only data inside [Desktop Entry] should be included
> -"Name=Example\n"
> -//empty lines
> -"\n"
> -" \n"
> -// make sure translations are included:
> -"Name[de_DE]=Beispiel\n"
> -// ignore comments:
> -"#Comment=Comment\n"
> -"  #Comment=Comment\n"
> -"Categories=foo;bar;a\\;b\n"
> -// As the case is significant, the keys Name and NAME are not 
> equivalent:
> -"CaseSensitive=ABC\n"
> -"CASESENSITIVE=abc\n"
> -// Space before and after the equals sign should be ignored:
> -"SpacesBeforeEq   =foo\n"
> -"SpacesAfterEq=   foo\n"
> -//  Space before and after the equals sign should be ignored; the = 
> sign is the actual delimiter.
> -// TODO: error in spec (spaces before and after the key??)
> -"   SpacesBeforeKey=foo\n"
> -"SpacesAfterKey   =foo\n"
> -// ignore trailing spaces
> -"TrailingSpaces=foo   \n"
> -// However spaces in the value are significant:
> -"SpacesInValue=Hello, World!\n"
> -//  The escape sequences \s, \n, \t, \r, and \\ are supported for 
> values of
> -// type string and localestring, meaning ASCII space, newline, tab,
> -// carriage return, and backslash, respectively:
> -"EscapeSequences=So\\sme esc\\nap\\te sequ\\rences\n" // 
> make sure that the last n is a literal n not a newline!
> -// the standard keys that are used by plugins, make sure correct 
> types are used:
> -"X-KDE-PluginInfo-Category=Examples\n" // string key
> -"X-KDE-PluginInfo-Version=1.0\n"
> -// The multiple values should be separated by a semicolon and the 
> value of the key
> -// may be optionally terminated by a semicolon. Trailing empty 
> strings must always
> -// be terminated with a semicolon. Semicolons in these values need 
> to be escaped using \;.
> -"X-KDE-PluginInfo-Depends=foo,bar,esc\\,aped\n" // string list 
> key
> -"X-KDE-ServiceTypes=\n" // empty string list
> -"X-KDE-PluginInfo-EnabledByDefault=true\n" // bool key
> -// now start a new group
> -"[New Group]\n"
> -"InWrongGroup=true\n";
> +// include an insignificant group
> +"[Some Group]\n"

not necessary to change indentation in this patch.

> desktoptojsontest.cpp:167
>  QByteArray kdevInput =
> -"[Desktop Entry]\n"
> -"Type = Service\n"
> -"Icon=text-x-c++src\n"
> -"Exec=blubb\n"
> -"Comment=C/C++ Language Support\n"
> -"Comment[fr]=Prise en charge du langage C/C++\n"
> -"Comment[it]=Supporto al linguaggio C/C++\n"
> -"Name=C++ Support\n"
> -"Name[fi]=C++-tuki\n"
> -"Name[fr]=Prise en charge du C++\n"
> -"GenericName=Language Support\n"
> -"GenericName[sl]=Podpora jeziku\n"
> -"ServiceTypes=KDevelop/NonExistentPlugin\n"
> -"X-KDE-Library=kdevcpplanguagesupport\n"
> -"X-KDE-PluginInfo-Name=kdevcppsupport\n"
> -"X-KDE-PluginInfo-Category=Language Support\n"
> -"X-KDevelop-Version=1\n"
> -"X-KDevelop-Language=C++\n"
> -"X-KDevelop-Args=CPP\n"
> -"X-KDevelop-Interfaces=ILanguageSupport\n"
> -
> "X-KDevelop-SupportedMimeTypes=text/x-chdr,text/x-c++hdr,text/x-csrc,text/x-c++src\n"
> -"X-KDevelop-Mode=NoGUI\n"
> -"X-KDevelop-LoadMode=AlwaysOn";
> +"[Desktop Entry]\n"
> +"Type = Service\n"

don't change indent please

> kaboutdatatest.cpp:184
>  QLatin1String("copyright"), QLatin1String("hello world"),
> -"http://www.koffice.org;);
> +QString::fromLatin1("http://www.koffice.org;));
>  QCOMPARE(data.organizationDomain(), QString::fromLatin1("koffice.org"));

QStringLiteral doen't work here ?

> kprocesstest.cpp:93
> +p.setShellCommand(QStringLiteral("true || false"));
> +QCOMPARE(p.program(), QStringList() << QString::fromLatin1("/bin/sh") << 
> QString::fromLatin1("-c")
> + << QString::fromLatin1("true || false"));

QStringLiteral

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun added a comment.


  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::appendTo(const char 
[16], QChar*&)’
  
QConcatenable::appendTo(p.a, out);
  ^
  
  same kind of errors at 69, 71, 73

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 24199.

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24198=24199

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginloadertest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kprocesstest.cpp
  autotests/kshelltest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent added a comment.


  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
  "

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 24198.
velurimithun added a comment.


  Update the code using QStringLiteral

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24197=24198

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

AFFECTED FILES
  autotests/kpluginloadertest.cpp
  autotests/kprocesstest.cpp

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 24197.
velurimithun added a comment.


  Changes are made using QStringLiteral

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24175=24197

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

AFFECTED FILES
  autotests/kpluginloadertest.cpp
  autotests/kprocesstest.cpp

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Laurent Montel]

mlaurent requested changes to this revision.
mlaurent added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kaboutdatatest.cpp:48
>  
> -static const char AppName[] ="app";
> -static const char ProgramName[] ="ProgramName";
> -static const char ProgramIconName[] ="program-icon";
> -static const char Version[] ="Version";
> -static const char ShortDescription[] =   "ShortDescription";
> -static const char CopyrightStatement[] = "CopyrightStatement";
> -static const char Text[] =   "Text";
> -static const char HomePageAddress[] ="http://test.no.where/;;
> -static const char HomePageSecure[] = "https://test.no.where/;;
> -static const char OrganizationDomain[] = "no.where";
> -static const char BugsEmailAddress[] =   "b...@no.else";
> -static const char LicenseText[] ="free to write, reading forbidden";
> -static const char LicenseFileName[] ="testlicensefile";
> -static const char LicenseFileText[] ="free to write, reading forbidden, 
> in the file";
> +static const QString AppName =QString::fromLatin1("app");
> +static const QString ProgramName =QString::fromLatin1("ProgramName");

no we need to keep const char ...[] and use QString::fromLatin1(...) in method 
as it's already done previously

> kautosavefiletest.cpp:83
>  {
> -QUrl normalFile = 
> QUrl::fromLocalFile(QDir::temp().absoluteFilePath("test directory/tîst 
> me.txt"));
> +QUrl normalFile = 
> QUrl::fromLocalFile(QDir::temp().absoluteFilePath(QLatin1String("test 
> directory/tîst me.txt")));
>  

QStringLiteral(...)

> kformattest.cpp:36
>  
> -QCOMPARE(format.formatByteSize(0), QString("0 B"));
> -QCOMPARE(format.formatByteSize(50), QString("50 B"));
> -QCOMPARE(format.formatByteSize(500), QString("500 B"));
> -QCOMPARE(format.formatByteSize(5000), QString("4.9 KiB"));
> -QCOMPARE(format.formatByteSize(5), QString("48.8 KiB"));
> -QCOMPARE(format.formatByteSize(50), QString("488.3 KiB"));
> -QCOMPARE(format.formatByteSize(500), QString("4.8 MiB"));
> -QCOMPARE(format.formatByteSize(5000), QString("47.7 MiB"));
> -QCOMPARE(format.formatByteSize(5), QString("476.8 MiB"));
> +QCOMPARE(format.formatByteSize(0), QLatin1String("0 B"));
> +QCOMPARE(format.formatByteSize(50), QLatin1String("50 B"));

QStringLiteral here and others.

> kformattest.cpp:308
>  testDate = QDate::currentDate().addDays(-7);
>  QCOMPARE(format.formatRelativeDate(testDate, QLocale::LongFormat),
> + QString::fromLatin1("Last 
> %1").arg(QLocale::c().dayName(testDate.dayOfWeek(), QLocale::LongFormat)));

QStringLiteral here too

> kpluginfactorytest.cpp:34
>  {
> -KPluginLoader multiplugin("multiplugin");
> +KPluginLoader multiplugin(QLatin1String("multiplugin"));
>  KPluginFactory *factory = multiplugin.factory();

QStringLiteral(...)

> kpluginmetadatatest.cpp:49
>  {
> -QString location = KPluginLoader::findPlugin("jsonplugin");
> +QString location = 
> KPluginLoader::findPlugin(QLatin1String("jsonplugin"));
>  QVERIFY2(!location.isEmpty(),"Could not find jsonplugin");

Same

> kpluginmetadatatest.cpp:207
>  QTest::ignoreMessage(QtWarningMsg, "Expected JSON property 
> \"String\" to be a string list. Treating it as a list with a single entry: 
> \"foo\" ");
> -QCOMPARE(KPluginMetaData::readStringList(jo, "String"), 
> QStringList("foo"));
> -QCOMPARE(KPluginMetaData::readStringList(jo, "OneArrayEntry"), 
> QStringList("foo"));
> +QCOMPARE(KPluginMetaData::readStringList(jo, 
> QLatin1String("String")), QStringList(QLatin1String("foo")));
> +QCOMPARE(KPluginMetaData::readStringList(jo, 
> QLatin1String("OneArrayEntry")), QStringList(QLatin1String("foo")));

Same

> kshelltest.cpp:58
>  QString me(KUser().loginName());
> -QCOMPARE(KShell::tildeExpand("~"), QDir::homePath());
> -QCOMPARE(KShell::tildeExpand("~/dir"), QString(QDir::homePath() + 
> "/dir"));
> -QCOMPARE(KShell::tildeExpand('~' + me), myHomePath());
> -QCOMPARE(KShell::tildeExpand('~' + me + "/dir"), QString(myHomePath() + 
> "/dir"));
> +QCOMPARE(KShell::tildeExpand(QString::fromLatin1("~")), 
> QDir::homePath());
> +QCOMPARE(KShell::tildeExpand(QString::fromLatin1("~/dir")), 
> QString(QDir::homePath() + QString::fromLatin1("/dir")));

QStringLiteral...

> kshelltest.cpp:84
>  #else
> -QCOMPARE(KShell::quoteArg("a space"), QString("'a space'"));
> +QCOMPARE(KShell::quoteArg(QString::fromLatin1("a space")), 
> QString::fromLatin1("'a space'"));
>  #endif

you can change QString::fromLatin1(...) by QStringLiteral in all code that you 
modified

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun updated this revision to Diff 24175.
velurimithun added a comment.


  - use QLating1String

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9420?vs=24143=24175

BRANCH
  master

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kaboutdatatest.cpp
  autotests/kautosavefiletest.cpp
  autotests/kdelibs4migrationtest.cpp
  autotests/kformattest.cpp
  autotests/kpluginfactorytest.cpp
  autotests/kpluginmetadatatest.cpp
  autotests/kshelltest.cpp
  autotests/ktexttohtmltest.cpp
  autotests/kusertest.cpp
  autotests/multiplugin.cpp

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII )

[Veluri Mithun]

velurimithun retitled this revision from "Compile commenting 
remove_defintion(QT_NO_CAST_FROM_ASCII defination)" to "Compile commenting 
remove_defintion(QT_NO_CAST_FROM_ASCII )".

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII defination)

[Laurent Montel]

mlaurent added a comment.


  Arguiment from method use QString and not QLatin1String()
  QString::fromLatin1 returns a QString

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII defination)

[Laurent Montel]

mlaurent added a comment.


  KAboutData aboutData2(QString::fromLatin1(AppName2), 
QString::fromLatin1(ProgramName2), QString::fromLatin1(Version2));
  
  use QString::fromLatin1 is will work

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII defination)

[Veluri Mithun]

velurimithun added a comment.


  
/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);
 ^

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII defination)

[Laurent Montel]

mlaurent requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII defination)

[Laurent Montel]

mlaurent added a comment.


  Could you paste you compile error here please.

INLINE COMMENTS

> CMakeLists.txt:1
> -remove_definitions(-DQT_NO_CAST_FROM_ASCII)
> +#remove_definitions(-DQT_NO_CAST_FROM_ASCII)
>  

Better to remove it.

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII defination)

[Veluri Mithun]

velurimithun added a comment.


  Errors in Kaboutdataappplicationdatatest file

REPOSITORY
  R244 KCoreAddons

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

To: velurimithun, mlaurent
Cc: #frameworks

D9420: Compile commenting remove_defintion(QT_NO_CAST_FROM_ASCII defination)

[Veluri Mithun]

velurimithun created this revision.
velurimithun added a reviewer: mlaurent.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

TEST PLAN
  Use QStringLiteral or QLatin1String for normal strings

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/desktoptojsontest.cpp
  autotests/kaboutdataapplicationdatatest.cpp
  autotests/kdelibs4migrationtest.cpp

To: velurimithun, mlaurent
Cc: #frameworks