From 695b08c5462c5ea36231c4d0938fccba1e0ab1e2 Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Wed, 2 Aug 2023 07:25:48 -0400 Subject: [PATCH 1/7] Change Index and TextPos to use enum constructors for extents The static member variable version was flawed as it suffered from SIOF, since TextPos' static END made use of Index's static LAST. Often, END would be initialized before LAST resulting in END being invalid/garbage. This approach, while slightly less terse, prevents that issue. The only other option is to use static member functions that return a type constructed to be that extent, which would enact RAII. The interface would look like: Index32::last(); instead of Index32(Index32::Last). --- lib/core/include/qx/core/qx-index.h | 54 ++++++++++++++++------------- lib/core/src/qx-index.dox | 34 ++++++++---------- lib/io/include/qx/io/qx-common-io.h | 10 +++--- lib/io/include/qx/io/qx-textpos.h | 12 +++++-- lib/io/src/qx-common-io.cpp | 26 +++++++------- lib/io/src/qx-common-io_p.cpp | 4 +-- lib/io/src/qx-common-io_p.h | 2 +- lib/io/src/qx-textpos.cpp | 40 +++++++++++++++++---- lib/io/src/qx-textquery.cpp | 4 +-- 9 files changed, 111 insertions(+), 75 deletions(-) diff --git a/lib/core/include/qx/core/qx-index.h b/lib/core/include/qx/core/qx-index.h index 492ea766..4df1054d 100644 --- a/lib/core/include/qx/core/qx-index.h +++ b/lib/core/include/qx/core/qx-index.h @@ -22,12 +22,13 @@ class Index //-Class Types---------------------------------------------------------------------------------------------------- private: enum class Type {Null, End, Value}; - struct LastKey {}; -//-Class Members---------------------------------------------------------------------------------------------------- public: - static const Index FIRST; - static const Index LAST; + enum Extent + { + First, + Last + }; //-Instance Members---------------------------------------------------------------------------------------------------- private: @@ -35,19 +36,32 @@ class Index T mValue; //-Constructor---------------------------------------------------------------------------------------------- -private: - Index(LastKey) : - mType(Type::End), - mValue(std::numeric_limits::max()) - {} - public: - Index() : + constexpr Index() : mType(Type::Null), mValue(0) {} - Index(T value) : + constexpr Index(Extent e) + { + switch(e) + { + case First: + mType = Type::Value; + mValue = 0; + break; + + case Last: + mType = Type::End; + mValue = std::numeric_limits::max(); + break; + + default: + qCritical("Invalid extent"); + } + } + + constexpr Index(T value) : mType(Type::Value), mValue(value) { @@ -93,7 +107,7 @@ class Index if(other.mType == Type::End) return 0; else if(mType == Type::End) - return LAST; + return Index(Last); else return constrainedSub(mValue, other.mValue, 0); } @@ -103,7 +117,7 @@ class Index Index operator+(const Index& other) { return (mType == Type::End || other.mType == Type::End) ? - LAST : constrainedAdd(mValue, other.mValue, 0); + Index(Last) : constrainedAdd(mValue, other.mValue, 0); } @@ -117,7 +131,7 @@ class Index if(other.mType == Type::End) return mType == Type::End ? 1 : 0; else if(mType == Type::End) - return LAST; + return Index(Last); else return constrainedDiv(mValue, other.mValue, 0); } @@ -129,7 +143,7 @@ class Index if(mValue == 0 || other.mValue == 0) return 0; else if(mType == Type::End || other.mType == Type::End) - return LAST; + return Index(Last); else return constrainedMult(mValue, other.mValue, 0); @@ -289,14 +303,6 @@ class Index // friend T& operator*=(N& integer, const Index& index) { integer *= static_cast(index.mValue); return integer; } }; -//-Outer Class Definitions---------------------------------------------------------------------------------- -template - requires std::signed_integral -const Index Index::FIRST = Index(0); -template - requires std::signed_integral -const Index Index::LAST = Index(LastKey{}); - //-Outer Class Types---------------------------------------------------------------------------------------- typedef Index Index8; typedef Index Index16; diff --git a/lib/core/src/qx-index.dox b/lib/core/src/qx-index.dox index 25ab4e58..d25f21f7 100644 --- a/lib/core/src/qx-index.dox +++ b/lib/core/src/qx-index.dox @@ -17,7 +17,7 @@ namespace Qx * use -1 to mean 'none' in the context of searches. * * The Index type is designed to remedy this issue by providing a more distinct representation of a set's final index - * via the member variable Index::LAST, which for completeness is paired with Index::FIRST. The type also + * via the constructor Index(Extent), which can also be used to more explicitly reference the first index. The type also * can be null, providing further clarity for the 'none' case. * * To minimize unintentional interactions, Index only has comparison and arithmetic operators defined for when @@ -52,20 +52,18 @@ namespace Qx * @li 2) 'last' indices are greater than everything except other 'last' indices (highest possible value) */ -//-Class Members---------------------------------------------------------------------------------------------------- +//-Class Types---------------------------------------------------------------------------------------------------- //Public: /*! - * @var Index Index::FIRST + * @enum Index::Extent * - * Used to refer to the first index of a dataset. + * Used to refer to indicies of special significance. * - * This is equivalent to @c Index(0). - */ - -/*! - * @var Index Index::LAST + * @var Index::Extent Index::First + * The first index, equivalent to @c Index(0). * - * Used to refer to the last index of a dataset. + * @var Index::Extent Index::Last + * The last index. */ //-Constructor---------------------------------------------------------------------------------------------- @@ -76,6 +74,12 @@ namespace Qx * Creates an null index. */ +/*! + * @fn Index::Index(Extent e) + * + * Creates an index at the given extent @a e. + */ + /*! * @fn Index::Index(T value) * @@ -97,15 +101,7 @@ namespace Qx * * Returns @c true if the index represents the last index of an arbitrary dataset; otherwise returns @c false. * - * This is equivalent to *this == Index::LAST. - */ - -/*! - * @fn bool Index::isLast() const - * - * Returns @c true if the index represents the last index of an arbitrary dataset; otherwise returns @c false. - * - * This is equivalent to *this == Index::LAST. + * This is equivalent to *this == Index(Last). */ //-Operators------------------------------------------------------------------------------------------------------- diff --git a/lib/io/include/qx/io/qx-common-io.h b/lib/io/include/qx/io/qx-common-io.h index f267b937..e8888c09 100644 --- a/lib/io/include/qx/io/qx-common-io.h +++ b/lib/io/include/qx/io/qx-common-io.h @@ -62,10 +62,10 @@ QX_IO_EXPORT IoOpReport textFileAbsolutePosition(TextPos& textPos, QFile& textFi QX_IO_EXPORT IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const TextQuery& query, ReadOptions readOptions = NoReadOptions); QX_IO_EXPORT IoOpReport fileContainsString(bool& returnBuffer, QFile& textFile, const QString& query, Qt::CaseSensitivity caseSensitivity = Qt::CaseSensitive, bool allowSplit = false); QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos, int count, ReadOptions readOptions = NoReadOptions); -QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos = TextPos::START, TextPos endPos = TextPos::END, ReadOptions readOptions = NoReadOptions); -QX_IO_EXPORT IoOpReport readTextFromFile(QStringList& returnBuffer, QFile& textFile, Index32 startLine = 0, Index32 endLine = Index32::LAST, ReadOptions readOptions = NoReadOptions); -QX_IO_EXPORT IoOpReport writeStringToFile(QFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos::START, WriteOptions writeOptions = NoWriteOptions); -QX_IO_EXPORT IoOpReport writeStringToFile(QSaveFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos::START, WriteOptions writeOptions = NoWriteOptions); +QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos = TextPos(TextPos::Start), TextPos endPos = TextPos(TextPos::End), ReadOptions readOptions = NoReadOptions); +QX_IO_EXPORT IoOpReport readTextFromFile(QStringList& returnBuffer, QFile& textFile, Index32 startLine = 0, Index32 endLine = Index32(Index32::Last), ReadOptions readOptions = NoReadOptions); +QX_IO_EXPORT IoOpReport writeStringToFile(QFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos(TextPos::Start), WriteOptions writeOptions = NoWriteOptions); +QX_IO_EXPORT IoOpReport writeStringToFile(QSaveFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos(TextPos::Start), WriteOptions writeOptions = NoWriteOptions); QX_IO_EXPORT IoOpReport deleteTextFromFile(QFile& textFile, TextPos startPos, TextPos endPos); // Directory: @@ -81,7 +81,7 @@ QX_IO_EXPORT IoOpReport calculateFileChecksum(QString& returnBuffer, QFile& file QX_IO_EXPORT IoOpReport fileMatchesChecksum(bool& returnBuffer, QFile& file, QString checksum, QCryptographicHash::Algorithm hashAlgorithm); // Binary Based -QX_IO_EXPORT IoOpReport readBytesFromFile(QByteArray& returnBuffer, QFile& file, Index64 startPos = 0, Index64 endPos = Index64::LAST); +QX_IO_EXPORT IoOpReport readBytesFromFile(QByteArray& returnBuffer, QFile& file, Index64 startPos = 0, Index64 endPos = Index64(Index64::Last)); QX_IO_EXPORT IoOpReport writeBytesToFile(QFile& file, const QByteArray& bytes, WriteMode writeMode = Truncate, Index64 startPos = 0, WriteOptions writeOptions = NoWriteOptions); QX_IO_EXPORT IoOpReport writeBytesToFile(QSaveFile& file, const QByteArray& bytes, WriteMode writeMode = Truncate, Index64 startPos = 0, WriteOptions writeOptions = NoWriteOptions); } diff --git a/lib/io/include/qx/io/qx-textpos.h b/lib/io/include/qx/io/qx-textpos.h index 93bf4e7f..dfcfefc6 100644 --- a/lib/io/include/qx/io/qx-textpos.h +++ b/lib/io/include/qx/io/qx-textpos.h @@ -7,6 +7,10 @@ // Extra-component Includes #include "qx/core/qx-index.h" +// TODO: This class should probably be constexpr construable, though this +// would mean it cant have separate implementation files which is somewhat ugly. +// probably should reconsider this once using modules. + namespace Qx { @@ -14,8 +18,11 @@ class QX_IO_EXPORT TextPos { //-Class Types------------------------------------------------------------------------------------------------------ public: - static const TextPos START; - static const TextPos END; + enum Extent + { + Start, + End + }; //-Instance Variables------------------------------------------------------------------------------------------------ private: @@ -25,6 +32,7 @@ class QX_IO_EXPORT TextPos //-Constructor------------------------------------------------------------------------------------------------------- public: TextPos(); + TextPos(Extent e); TextPos(Index32 line, Index32 character); //-Instance Functions------------------------------------------------------------------------------------------------ diff --git a/lib/io/src/qx-common-io.cpp b/lib/io/src/qx-common-io.cpp index fc2173ef..d99ad33f 100644 --- a/lib/io/src/qx-common-io.cpp +++ b/lib/io/src/qx-common-io.cpp @@ -378,9 +378,9 @@ IoOpReport textFileLineCount(int& returnBuffer, QFile& textFile, bool ignoreTrai /*! * Converts any relative component of @a textPos to an absolute one. I.e. determines the actual line - * and or/character that Index32::LAST references for the given @a textFile, if present. + * and or/character that Index32(Index32::Last) references for the given @a textFile, if present. * - * If neither the line or character component of @a textPos contain the value Index32::LAST, + * If neither the line or character component of @a textPos contain the value Index32(Index32::Last), * @a textPos is left unchanged. * * @param[in,out] textPos The text position to translate into an absolute position. @@ -459,7 +459,7 @@ IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const // Query tracking TextPos trueStartPos = query.startPosition(); - TextPos currentPos = TextPos::START; + TextPos currentPos = TextPos(TextPos::Start); TextPos possibleMatch = TextPos(); int hitsSkipped = 0; QString::const_iterator queryIt = query.string().constBegin(); @@ -469,7 +469,7 @@ IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const QTextStream fileTextStream(&textFile); // Translate start position to absolute position - if(trueStartPos != TextPos::START) + if(trueStartPos != TextPos(TextPos::Start)) { IoOpReport translate = textFileAbsolutePosition(trueStartPos, textFile, readOptions.testFlag(IgnoreTrailingBreak)); if(translate.isFailure()) @@ -489,7 +489,7 @@ IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const QScopeGuard fileGuard([&textFile](){ textFile.close(); }); // Skip to start pos - if(trueStartPos != TextPos::START) + if(trueStartPos != TextPos(TextPos::Start)) { int line; // Skip to start line @@ -739,7 +739,7 @@ IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos star TextStream fileTextStream(&textFile); // Cover each possible range type - if(startPos == TextPos::START && endPos == TextPos::END) // Whole file is desired + if(startPos == TextPos(TextPos::Start) && endPos == TextPos(TextPos::End)) // Whole file is desired { returnBuffer = fileTextStream.readAll(); @@ -999,7 +999,7 @@ namespace // Fill beforeNew TextPos beforeEnd = TextPos(startPos.line(), startPos.character() - 1); - IoOpReport readBefore = readTextFromFile(beforeNew, auxFile, TextPos::START, beforeEnd); + IoOpReport readBefore = readTextFromFile(beforeNew, auxFile, TextPos(TextPos::Start), beforeEnd); if(readBefore.isFailure()) return readBefore; @@ -1178,27 +1178,27 @@ IoOpReport deleteTextFromFile(QFile& textFile, TextPos startPos, TextPos endPos) IoOpReport transientReport; // Determine beforeDeletion - if(startPos == TextPos::START) // (0,0) + if(startPos == TextPos(TextPos::Start)) // (0,0) beforeDeletion = u""_s; else if(startPos.character().isLast()) { - transientReport = readTextFromFile(beforeDeletion, textFile, TextPos::START, startPos); + transientReport = readTextFromFile(beforeDeletion, textFile, TextPos(TextPos::Start), startPos); beforeDeletion.chop(1); } else - transientReport = readTextFromFile(beforeDeletion, textFile, TextPos::START, TextPos(startPos.line(), startPos.character() - 1)); + transientReport = readTextFromFile(beforeDeletion, textFile, TextPos(TextPos::Start), TextPos(startPos.line(), startPos.character() - 1)); // Check for transient errors if(!transientReport.isNull() && transientReport.result() != IO_SUCCESS) return IoOpReport(IO_OP_WRITE, transientReport.result(), textFile); // Determine afterDeletion - if(endPos == TextPos::END) + if(endPos == TextPos(TextPos::End)) afterDeletion = u""_s; else if(endPos.character().isLast()) - transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line() + 1, 0), TextPos::END); + transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line() + 1, 0), TextPos(TextPos::End)); else - transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line(), endPos.character() + 1), TextPos::END); + transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line(), endPos.character() + 1), TextPos(TextPos::End)); // Check for transient errors if(transientReport.result() != IO_SUCCESS) diff --git a/lib/io/src/qx-common-io_p.cpp b/lib/io/src/qx-common-io_p.cpp index 3961b2ee..b240c34e 100644 --- a/lib/io/src/qx-common-io_p.cpp +++ b/lib/io/src/qx-common-io_p.cpp @@ -132,10 +132,10 @@ IoOpReport writePrep(const QFileInfo& fileInfo, WriteOptions writeOptions) void matchAppendConditionParams(WriteMode& writeMode, TextPos& startPos) { // Match append condition parameters - if(startPos == TextPos::END) + if(startPos == TextPos(TextPos::End)) writeMode = Append; else if(writeMode == Append) - startPos = TextPos::END; + startPos = TextPos(TextPos::End); } /*! @endcond */ diff --git a/lib/io/src/qx-common-io_p.h b/lib/io/src/qx-common-io_p.h index 10a5fb35..26ddb4b2 100644 --- a/lib/io/src/qx-common-io_p.h +++ b/lib/io/src/qx-common-io_p.h @@ -37,7 +37,7 @@ void matchAppendConditionParams(WriteMode& writeMode, Index& startPos) if(startPos.isLast()) writeMode = Append; else if(writeMode == Append) - startPos = Index::LAST; + startPos = Index(Index::Last); } /*! @endcond */ } diff --git a/lib/io/src/qx-textpos.cpp b/lib/io/src/qx-textpos.cpp index b471457a..6b6198c5 100644 --- a/lib/io/src/qx-textpos.cpp +++ b/lib/io/src/qx-textpos.cpp @@ -15,22 +15,24 @@ namespace Qx * @brief The TextPos class is used to represent an offset within a text file in terms of lines and characters. */ -//-Class Variables----------------------------------------------------------------------------------------------- +//-Class Types----------------------------------------------------------------------------------------------- //Public: /*! + * @enum Index::Extent + * + * Used to refer to text positions of special significance. + * + * @var Index::Extent Index::First * A text position representing the start of a file. * * Equivalent to @c TextPos(0,0). - */ -const TextPos TextPos::START = TextPos(0,0); // Initialization of constant reference TextPos - -/*! + * + * @var Index::Extent Index::Last * A text position representing the end file. * - * Equivalent to @c TextPos(Index32::LAST,Index32::LAST). + * Equivalent to @c TextPos(Index32(Index32::Last), Index32(Index32::Last)). */ -const TextPos TextPos::END = TextPos(Index32::LAST, Index32::LAST); // Initialization of constant reference TextPos //-Constructor--------------------------------------------------------------------------------------------------- //Public: @@ -43,6 +45,30 @@ TextPos::TextPos() : mCharacter(Index32()) {} +//TODO: Consider creating Qx::Extent (with Start/End) in qx-global and just use +// that for Index and TextPos. +/*! + * Creates a text position at the given extent @a e. + */ +TextPos::TextPos(Extent e) +{ + switch(e) + { + case Start: + mLine = 0; + mCharacter = 0; + break; + + case End: + mLine = Index32(Index32::Last); + mCharacter = Index32(Index32::Last); + break; + + default: + qCritical("Invalid extent"); + } +} + /*! * Creates a text position that points to @a line and @a character. */ diff --git a/lib/io/src/qx-textquery.cpp b/lib/io/src/qx-textquery.cpp index d6348b64..ae2e8dab 100644 --- a/lib/io/src/qx-textquery.cpp +++ b/lib/io/src/qx-textquery.cpp @@ -27,7 +27,7 @@ namespace Qx TextQuery::TextQuery(const QString& string, Qt::CaseSensitivity cs) : mString(string), mCaseSensitivity(cs), - mStartPos(TextPos::START), + mStartPos(TextPos(TextPos::Start)), mHitsToSkip(0), mHitLimit(-1), mAllowSplit(false) @@ -48,7 +48,7 @@ Qt::CaseSensitivity TextQuery::caseSensitivity() const { return mCaseSensitivity /*! * Returns the position from which the search should begin. * - * The default is @c TextPos::START. + * The default is @c TextPos(TextPos::Start). */ TextPos TextQuery::startPosition() const { return mStartPos; } From 0db75b13f6a7d40eeaf92afa6c23e27fd653be4e Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Wed, 2 Aug 2023 09:10:07 -0400 Subject: [PATCH 2/7] Add start of qx-common-io tests --- tests/io/CMakeLists.txt | 1 + tests/io/qx_common_io/CMakeLists.txt | 21 +++++ .../writeStringToFile/overwrite_expected.txt | 5 ++ .../writeStringToFile/overwrite_input.txt | 3 + .../writeStringToFile/overwrite_original.txt | 5 ++ tests/io/qx_common_io/tst_qx_common_io.cpp | 87 +++++++++++++++++++ 6 files changed, 122 insertions(+) create mode 100644 tests/io/qx_common_io/CMakeLists.txt create mode 100644 tests/io/qx_common_io/data/writeStringToFile/overwrite_expected.txt create mode 100644 tests/io/qx_common_io/data/writeStringToFile/overwrite_input.txt create mode 100644 tests/io/qx_common_io/data/writeStringToFile/overwrite_original.txt create mode 100644 tests/io/qx_common_io/tst_qx_common_io.cpp diff --git a/tests/io/CMakeLists.txt b/tests/io/CMakeLists.txt index e69de29b..60b740a1 100644 --- a/tests/io/CMakeLists.txt +++ b/tests/io/CMakeLists.txt @@ -0,0 +1 @@ +add_subdirectory(qx_common_io) \ No newline at end of file diff --git a/tests/io/qx_common_io/CMakeLists.txt b/tests/io/qx_common_io/CMakeLists.txt new file mode 100644 index 00000000..efbc5a9d --- /dev/null +++ b/tests/io/qx_common_io/CMakeLists.txt @@ -0,0 +1,21 @@ +include(OB/Test) + +ob_add_basic_standard_test( + TARGET_PREFIX "${TESTS_TARGET_PREFIX}" + TARGET_VAR test_target + LINKS + ${TESTS_COMMON_TARGET} + Qx::Io +) + +# Bundle test data +file(GLOB_RECURSE text_data + RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} + "data/*.*" +) + +qt_add_resources(${test_target} "tst_qx_common_io_data" + PREFIX "/" + FILES + ${text_data} +) diff --git a/tests/io/qx_common_io/data/writeStringToFile/overwrite_expected.txt b/tests/io/qx_common_io/data/writeStringToFile/overwrite_expected.txt new file mode 100644 index 00000000..79de78a8 --- /dev/null +++ b/tests/io/qx_common_io/data/writeStringToFile/overwrite_expected.txt @@ -0,0 +1,5 @@ +0123456789 +01Jishy +SaltSalivaMan +test456789 +0123456789 \ No newline at end of file diff --git a/tests/io/qx_common_io/data/writeStringToFile/overwrite_input.txt b/tests/io/qx_common_io/data/writeStringToFile/overwrite_input.txt new file mode 100644 index 00000000..dd91279c --- /dev/null +++ b/tests/io/qx_common_io/data/writeStringToFile/overwrite_input.txt @@ -0,0 +1,3 @@ +Jishy +SaltSalivaMan +test \ No newline at end of file diff --git a/tests/io/qx_common_io/data/writeStringToFile/overwrite_original.txt b/tests/io/qx_common_io/data/writeStringToFile/overwrite_original.txt new file mode 100644 index 00000000..a0124d34 --- /dev/null +++ b/tests/io/qx_common_io/data/writeStringToFile/overwrite_original.txt @@ -0,0 +1,5 @@ +0123456789 +0123456789 +0123456789 +0123456789 +0123456789 \ No newline at end of file diff --git a/tests/io/qx_common_io/tst_qx_common_io.cpp b/tests/io/qx_common_io/tst_qx_common_io.cpp new file mode 100644 index 00000000..3e7a512d --- /dev/null +++ b/tests/io/qx_common_io/tst_qx_common_io.cpp @@ -0,0 +1,87 @@ +// Qt Includes +#include + +// Qx Includes +#include + +// Test Includes +//#include + +class tst_qx_common_io : public QObject +{ + Q_OBJECT + +private: + QTemporaryDir mWriteDir; + +public: + tst_qx_common_io(); + +private slots: + // Init + // void initTestCase(); + // void initTestCase_data(); + // void cleanupTestCase(); + // void init() + // void cleanup(); + + // Test cases + void writeStringToFile_data(); + void writeStringToFile(); +}; + +// Setup +tst_qx_common_io::tst_qx_common_io() +{ + QVERIFY(mWriteDir.isValid()); +} + +// Cases +void tst_qx_common_io::writeStringToFile_data() +{ + QTest::addColumn("file"); + QTest::addColumn("input"); + QTest::addColumn("expected"); + + // TODO: Make this more generic with helper functions once more test files are added + + // Overwrite + QString overwritePath = mWriteDir.filePath("overwrite_file.txt"); + QVERIFY(QFile::copy(u":/data/writeStringToFile/overwrite_original.txt"_s, overwritePath)); + QFile::setPermissions(overwritePath, QFile::WriteOwner); // Required since copying from a file marked read-only + + QFile inputFile(u":/data/writeStringToFile/overwrite_input.txt"_s); + QVERIFY(inputFile.open(QIODevice::ReadOnly | QIODevice::Text)); + QString input = inputFile.readAll(); + inputFile.close(); + + QFile expectedFile(u":/data/writeStringToFile/overwrite_expected.txt"_s); + QVERIFY(expectedFile.open(QIODevice::ReadOnly | QIODevice::Text)); + QString expected = expectedFile.readAll(); + expectedFile.close(); + + QTest::newRow("Overwrite") << new QFile(overwritePath, this) << input << expected; +} + +void tst_qx_common_io::writeStringToFile() +{ + // Fetch data from test table + QFETCH(QFile*, file); + QFETCH(QString, input); + QFETCH(QString, expected); + + // Write to file + // TODO: Handle different arguments for writeStringToFile in _data + Qx::IoOpReport rp = Qx::writeStringToFile(*file, input, Qx::Overwrite, Qx::TextPos(1,2)); + QVERIFY(!rp.isFailure()); + + // Get file's new contents and compare + QVERIFY(file->open(QIODevice::ReadOnly | QIODevice::Text)); + QTextStream ts(file); // Handles proper conversion of \r\n to \n + QString newData = ts.readAll(); + QCOMPARE(newData, expected); + file->close(); +} + +QTEST_APPLESS_MAIN(tst_qx_common_io) +#include "tst_qx_common_io.moc" From e59355ed59fc582ac755a71bae2865d712ef4023 Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Tue, 8 Aug 2023 05:15:42 -0400 Subject: [PATCH 3/7] Rewrite TextPos comparisons using C++20 guidance of only == and <=> This also fixes ambiguity warnings introduced with C++20 due to the old operators not having proper const correctness. --- lib/io/include/qx/io/qx-textpos.h | 8 ++---- lib/io/src/qx-textpos.cpp | 45 +++++++++---------------------- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/lib/io/include/qx/io/qx-textpos.h b/lib/io/include/qx/io/qx-textpos.h index dfcfefc6..b26bcfdc 100644 --- a/lib/io/include/qx/io/qx-textpos.h +++ b/lib/io/include/qx/io/qx-textpos.h @@ -43,12 +43,8 @@ class QX_IO_EXPORT TextPos void setCharacter(Index32 character); bool isNull() const; - bool operator== (const TextPos& otherTextPos); - bool operator!= (const TextPos& otherTextPos); - bool operator> (const TextPos& otherTextPos); - bool operator>= (const TextPos& otherTextPos); - bool operator< (const TextPos& otherTextPos); - bool operator<= (const TextPos& otherTextPos); + bool operator==(const TextPos& other) const noexcept = default; + auto operator<=>(const TextPos& other) const noexcept; }; } diff --git a/lib/io/src/qx-textpos.cpp b/lib/io/src/qx-textpos.cpp index 6b6198c5..175b46e1 100644 --- a/lib/io/src/qx-textpos.cpp +++ b/lib/io/src/qx-textpos.cpp @@ -81,46 +81,27 @@ TextPos::TextPos(Index32 line, Index32 character) : //Public: /*! + * @fn bool TextPos::operator==(const TextPos& otherTextPos) const + * * Returns @c true if the line and character of this text position are the same as in @a otherTextPos; * otherwise returns @c false. */ -bool TextPos::operator==(const TextPos& otherTextPos) { return mLine == otherTextPos.mLine && mCharacter == otherTextPos.mCharacter; } - -/*! - * Returns @c true if either the line or character of this text position is different than in @a otherTextPos; - * otherwise returns @c false. - */ -bool TextPos::operator!= (const TextPos& otherTextPos) { return !(*this == otherTextPos); } /*! - * Returns @c true if this text position points to a further location than @a otherTextPos; - * otherwise returns @c false. + * Performs a three-way comparison between this text position and @a other. + * + * Returns: + * - @c <0 if this text position is less than @a other + * - @c 0 if this text position is equal to @a other + * - @c >0 if this text position is greater than @a other */ -bool TextPos::operator> (const TextPos& otherTextPos) +auto TextPos::operator<=>(const TextPos& other) const noexcept { - if(mLine == otherTextPos.mLine) - return mCharacter > otherTextPos.mCharacter; - else - return mLine > otherTextPos.mLine; -} - -/*! - * Returns @c true if this text position points to at least the same location as @a otherTextPos; - * otherwise returns @c false. - */ -bool TextPos::operator>= (const TextPos& otherTextPos) { return *this == otherTextPos || *this > otherTextPos; } - -/*! - * Returns @c true if this text position points to a closer location than @a otherTextPos; - * otherwise returns @c false. - */ -bool TextPos::operator< (const TextPos& otherTextPos) { return !(*this >= otherTextPos); } + if (auto c = mLine <=> other.mLine; c != 0) + return c; -/*! - * Returns @c true if this text position points to at most the same location as @a otherTextPos; - * otherwise returns @c false. - */ -bool TextPos::operator<= (const TextPos& otherTextPos) { return !(*this > otherTextPos); } + return mCharacter <=> other.mCharacter; +} /*! * Returns the line that the text position is pointing to. From b49f6e64e562a5fe6cddda27b7276e920701ff27 Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Tue, 8 Aug 2023 05:51:20 -0400 Subject: [PATCH 4/7] Fix Index & TextPos Extent documentation --- lib/core/src/qx-index.dox | 3 ++- lib/io/src/qx-textpos.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/core/src/qx-index.dox b/lib/core/src/qx-index.dox index d25f21f7..e919f3f0 100644 --- a/lib/core/src/qx-index.dox +++ b/lib/core/src/qx-index.dox @@ -54,8 +54,9 @@ namespace Qx //-Class Types---------------------------------------------------------------------------------------------------- //Public: +// TODO: Report this as a Doxygen bug, likely should need to be Index::Extent /*! - * @enum Index::Extent + * @enum Index::Extent * * Used to refer to indicies of special significance. * diff --git a/lib/io/src/qx-textpos.cpp b/lib/io/src/qx-textpos.cpp index 175b46e1..cbf2fc8d 100644 --- a/lib/io/src/qx-textpos.cpp +++ b/lib/io/src/qx-textpos.cpp @@ -19,7 +19,7 @@ namespace Qx //Public: /*! - * @enum Index::Extent + * @enum TextPos::Extent * * Used to refer to text positions of special significance. * From 565d252dc6b4b277e7ae1d3652c440aec0f9ba15 Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Tue, 8 Aug 2023 10:36:38 -0400 Subject: [PATCH 5/7] Fix Index/TextPos return type Needs to be explicit so they can be used before they are defined in other translation units. --- lib/core/include/qx/core/qx-index.h | 2 +- lib/core/src/qx-index.dox | 2 +- lib/io/include/qx/io/qx-textpos.h | 2 +- lib/io/src/qx-textpos.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/core/include/qx/core/qx-index.h b/lib/core/include/qx/core/qx-index.h index 4df1054d..4da655cf 100644 --- a/lib/core/include/qx/core/qx-index.h +++ b/lib/core/include/qx/core/qx-index.h @@ -84,7 +84,7 @@ class Index // template requires std::integral // bool operator==(const N& integer) const { return mType == Type::Value && static_cast(mValue) == integer; } - auto operator<=>(const Index& other) const noexcept { + std::strong_ordering operator<=>(const Index& other) const noexcept { switch(mType) { case Type::Null: diff --git a/lib/core/src/qx-index.dox b/lib/core/src/qx-index.dox index e919f3f0..3d1a329a 100644 --- a/lib/core/src/qx-index.dox +++ b/lib/core/src/qx-index.dox @@ -115,7 +115,7 @@ namespace Qx */ /*! - * @fn auto Index::operator<=>(const Index& other) const noexcept + * @fn std::strong_ordering Index::operator<=>(const Index& other) const noexcept * * Performs a three-way comparison between this index and @a other. * diff --git a/lib/io/include/qx/io/qx-textpos.h b/lib/io/include/qx/io/qx-textpos.h index b26bcfdc..8b81142d 100644 --- a/lib/io/include/qx/io/qx-textpos.h +++ b/lib/io/include/qx/io/qx-textpos.h @@ -44,7 +44,7 @@ class QX_IO_EXPORT TextPos bool isNull() const; bool operator==(const TextPos& other) const noexcept = default; - auto operator<=>(const TextPos& other) const noexcept; + std::strong_ordering operator<=>(const TextPos& other) const noexcept; }; } diff --git a/lib/io/src/qx-textpos.cpp b/lib/io/src/qx-textpos.cpp index cbf2fc8d..9f380c36 100644 --- a/lib/io/src/qx-textpos.cpp +++ b/lib/io/src/qx-textpos.cpp @@ -95,7 +95,7 @@ TextPos::TextPos(Index32 line, Index32 character) : * - @c 0 if this text position is equal to @a other * - @c >0 if this text position is greater than @a other */ -auto TextPos::operator<=>(const TextPos& other) const noexcept +std::strong_ordering TextPos::operator<=>(const TextPos& other) const noexcept { if (auto c = mLine <=> other.mLine; c != 0) return c; From 90d0e733533f1541b9f00bb666a8c6693bf62948 Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Wed, 9 Aug 2023 07:54:51 -0400 Subject: [PATCH 6/7] Fix common-io test on Linux --- tests/io/qx_common_io/tst_qx_common_io.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/io/qx_common_io/tst_qx_common_io.cpp b/tests/io/qx_common_io/tst_qx_common_io.cpp index 3e7a512d..df7a4217 100644 --- a/tests/io/qx_common_io/tst_qx_common_io.cpp +++ b/tests/io/qx_common_io/tst_qx_common_io.cpp @@ -48,7 +48,8 @@ void tst_qx_common_io::writeStringToFile_data() // Overwrite QString overwritePath = mWriteDir.filePath("overwrite_file.txt"); QVERIFY(QFile::copy(u":/data/writeStringToFile/overwrite_original.txt"_s, overwritePath)); - QFile::setPermissions(overwritePath, QFile::WriteOwner); // Required since copying from a file marked read-only + auto p = QFile::WriteOwner | QFile::ReadOwner | QFile::WriteUser | QFile::ReadUser; + QFile::setPermissions(overwritePath, p); // Required since copying from a file marked read-only QFile inputFile(u":/data/writeStringToFile/overwrite_input.txt"_s); QVERIFY(inputFile.open(QIODevice::ReadOnly | QIODevice::Text)); @@ -73,7 +74,7 @@ void tst_qx_common_io::writeStringToFile() // Write to file // TODO: Handle different arguments for writeStringToFile in _data Qx::IoOpReport rp = Qx::writeStringToFile(*file, input, Qx::Overwrite, Qx::TextPos(1,2)); - QVERIFY(!rp.isFailure()); + QVERIFY2(!rp.isFailure(), qPrintable(rp.outcomeInfo())); // Get file's new contents and compare QVERIFY(file->open(QIODevice::ReadOnly | QIODevice::Text)); From 7d530becbe927ad2b701ae1701c02adbc86730d6 Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Wed, 9 Aug 2023 07:57:50 -0400 Subject: [PATCH 7/7] Bump --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ad3292c3..dcebc682 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ cmake_minimum_required(VERSION 3.23.0...3.26.0) # avoided and only used for hotfixes. DON'T USE TRAILING # ZEROS IN VERSIONS project(Qx - VERSION 0.5.1.1 + VERSION 0.5.2 LANGUAGES CXX DESCRIPTION "Qt Extensions Library" )