"Cleaned up" Qt6 version without legacy (based on #66)

[markusdd]

As per the excellent PR #66 of @cristeab I picked up on the discussion and did a code cleanup by removing the legacy and running the whole project through clang formatting.

Tests pass for me on Windows using Qt 6.0.1 .

This can either be made the new master and Qt4/5 version gets preserved on a branch/tag or this gets it's own branch.

Thank you for this excellent useful project!

[markusdd]

still working on the CI scripts

[markusdd]

suggestion: for now, move QT6 build also to appveyor. There are no QT6 images on travis yet, so this setup would be painful.

Appveyors Ubuntu images have support though: https://www.appveyor.com/docs/linux-images-software/#qt

[markusdd]

QStringRef was deprecated in Qt6, so I ported the reader to QStringView, which also gives the startsWith and endsWith functions.

Maybe the reader should also make use of the dissect-functions of QStringView, might make the logic much more compact, but this is for another PR

[markusdd]

getting a negative exit code from tests now without any error output...how to debug this?

[markusdd]

Ok, Powershell problem in appveyor. So the error happens here:

$  /usr/bin/env c:\\Users\\Kraus\\.vscode\\extensions\\ms-vscode.cpptools-1.2.2\\debugAdapters\\bin\\WindowsDebugLauncher.exe --stdin=Microsoft-MIEngine-In-kjyol5qv.wi4 --stdout=Microsoft-MIEngine-Out-lpht5wfw.qar --stderr=Microsoft-MIEngine-Error-2dldflsp.siv --pid=Microsoft-MIEngine-Pi
d-1thmouen.jsa --dbgExe=E:/Qt/Tools/mingw810_64/bin/gdb.exe --interpreter=mi 
********* Start testing of TestStringData *********
Config: Using QtTest library 6.0.1, Qt 6.0.1 (x86_64-little_endian-llp64 shared (dynamic) release build; by GCC 8.1.0), windows 10
PASS   : TestStringData::initTestCase()
PASS   : TestStringData::testCreation()
PASS   : TestStringData::testAddEmptyRow()
PASS   : TestStringData::testAddOneRow()
PASS   : TestStringData::testAddOneRowUsingOneString()
PASS   : TestStringData::testAddRows()
PASS   : TestStringData::testClearEmptyData()
PASS   : TestStringData::testClearNotEmptyData()      
QFATAL : TestStringData::testInsertRows() ASSERT failure in QList<T>::insert: "index out of range", file E:/Qt/6.0.1/mingw81_64/include/QtCore/qlist.h, line 807
FAIL!  : TestStringData::testInsertRows() Received a fatal error.
Unknown file(0) : failure location
Totals: 8 passed, 1 failed, 0 skipped, 0 blacklisted, 68130ms    
********* Finished testing of TestStringData *********

Is this intentional to insert at Row 100? What is the expected behavior?

[markusdd]

ok so both CIs are now running, Ubuntu has only 6.0.0 which has another issue, but that'll be fixed soon when appveyor installs 6.0.1.

But I still do not get the StringData testcase, how has this ever worked adding an item at position 100 which is out of bounds?

[markusdd]

@iamantony so it is finally done after fighting the CI... I have ported everything to appveyor as travis has no proper Qt6 env yet. Everything is passing now for Windows and Linux. also thanks to @mhoeher who helped me fix a regression on QList handling which has changed from Qt5 and caused errors when inserting at a non-existing index position.

The reason for using bash.exe in appveyor even in Windows is that powershell produces no stdout when running the tests and exits with a bogus error code without any way to debug it (this cost me hours to figure out...).

Hope this helps. (in the meantime I'll try to also get a CI running for mac)

[markusdd]

ok, macOS pipeline is also clean

ready to merge I'd say.

[markusdd]

@iamantony are you still maintaining this?

[iamantony]

@markusdd thanks for your pull request! I really appreciate your time and effort to make qtcsv great again))

Files to remove:

• .vscode/c_cpp_properties.json
• qtcsv.code-workspace

Issues:

• most of the changes are about style. I understand that we have different preferences and that is OK, but in pull request about switching to Qt6 I would prefer to see only functionality changes
• #include <QtGlobal> - that should be avoided

So what have really changed in the code:

• QStringList -> QList<QString>
• QTextCodec -> QStringConverter
• QStringRef -> QStringView

Let's assume that we're allowed to change API of the library but still want to support Qt4 - Qt6. Could we do that?

• QStringList -> QList<QString> - that's fine, supported by Qt4 - Qt6
• QTextCodec -> QStringConverter - we can introduce a converter class with it's own enum with encodings. It would have a method that accepts QTextSteam object and set requested codec using setCodec() or setEncoding() depending on the Qt version.
• QStringRef -> QStringView - it seems that to eliminate dependency on Qt version we should switch to more general approach. For example, we can use const QString& for references and std:: algorithms for string comparison.

[markusdd]

as for your last point, I started out from #66 and understood the main branch at Qt6 would be preferred while conserving Qt4/5 on a seperate one.

admittedly, I am not a Qt expert (yet) so please feel free to modify this PR to your liking, due to my day job I have limited capacity to look into this right now, but of course I would like to see this get merged eventually as I've actually spent quite a bit of time doing the port.

I will look into reverting the formatting changes nevertheless, they just came formt he standard Clang-autoformatting I use in VS Code.

[iamantony]

Ok, I'll merge your PR in some way or another)) I finally decided to switch project to Qt6.

[markusdd]

So I have modified the autoformat to match your bracket and line-length and indentation style. As it was not 100% consistent there is still some diffs due to it, but now it is at least the exact same everywhere.

I used this for Clang Format: { BasedOnStyle: Microsoft, AllowShortFunctionsOnASingleLine: Empty, NamespaceIndentation: All, IndentWidth: 4, PointerAlignment: Left, DerivePointerAlignment: false, ColumnLimit: 80, BinPackArguments : false, BinPackParameters: false}

The windows pipeline (once again) has an issue, I have to check.

[markusdd]

Ok, they switched up their CMake version, it works again now

[iamantony]

@markusdd thanks again for your work! Code is merged into qt6 branch which later will be merged into master.

[markusdd]

sure, my pleasure!