Skip to content

Commit de18b26

Browse files
Address CI feedback: docs and clang-format for diff feature
Combined slice of upstream commits 24ccdfd (CI feedback) and ec27e91 (clang-format) restricted to the diff feature's surface area: - Doxygen comments for public members of SqlSchemaDiff (ColumnDiff, TableDiff) and SqlDataDiff (RowDiff, TableDataDiff, DiffProgressEvent), plus SecretResolver move ops. - Replace \\ref with backticks in SqlSchemaDiff/SqlDataDiff doc comments so doxygen stops complaining about unresolved references. - Exclude src/Lightweight/tui/ from doxygen — vendored code follows the upstream comment style. - Migrate \`.find(x) != std::string::npos\` to \`.contains(x)\` (and the inverse) across PR-introduced code (ProfileStore.cpp, SqlSchema.cpp, ConfigProfileStoreTests.cpp, SecretResolverTests.cpp), satisfying clang-tidy 22's readability-container-contains check. - clang-format-22 across modified C++ files (tui/, Secrets/, dbtool main.cpp). No behavioral changes — purely doc / lint / whitespace. Signed-off-by: Christian Parpart <christian@parpart.family>
1 parent f727bdb commit de18b26

17 files changed

Lines changed: 211 additions & 117 deletions

.github/workflows/build.yml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,27 @@ jobs:
110110
- name: "install dependencies"
111111
run: sudo apt install -y cmake ninja-build catch2 unixodbc-dev sqlite3 libsqlite3-dev libsqliteodbc uuid-dev libyaml-cpp-dev gcc-14 libtbb-dev libzip-dev
112112
- name: "cmake"
113+
# ENABLE_TIDY=OFF: this job invokes `run-clang-tidy` directly after
114+
# configure; we don't want CMAKE_CXX_CLANG_TIDY also wired up
115+
# because that runs clang-tidy on libunicode's own sources during
116+
# the codegen build step below — and libunicode predates clang-tidy
117+
# 22's pedantic warning set.
113118
run: |
114119
cmake --preset clang-debug \
115120
-D CMAKE_CXX_COMPILER=clang++-${{ env.CLANG_TOOLS_VERSION }} \
116121
-D CMAKE_C_COMPILER=clang-${{ env.CLANG_TOOLS_VERSION }} \
122+
-D ENABLE_TIDY=OFF \
117123
-B build
124+
- name: "build libunicode codegen (UCD tables)"
125+
# `unicode_ucd` is the libunicode static library whose source list
126+
# includes the auto-generated `ucd_enums.h` and friends. Building it
127+
# (only it) materialises those headers so that clang-tidy can parse
128+
# `<libunicode/codepoint_properties.h>` transitively included from
129+
# our vendored `tui/MarkdownTable.cpp`. Without this step, clang-tidy
130+
# aborts with `'libunicode/ucd_enums.h' file not found`.
131+
run: cmake --build build --target unicode_ucd
118132
- name: "run clang-tidy"
119-
run: run-clang-tidy-${{ env.CLANG_TOOLS_VERSION }} -p ./build -clang-tidy-binary clang-tidy-${{ env.CLANG_TOOLS_VERSION }} -config-file .clang-tidy -header-filter='^(?!(.*/)?stdexec-src/|(.*/)?libzip-src/).*$' -source-filter='^(?!.*_deps/).*$'
133+
run: run-clang-tidy-${{ env.CLANG_TOOLS_VERSION }} -p ./build -clang-tidy-binary clang-tidy-${{ env.CLANG_TOOLS_VERSION }} -config-file .clang-tidy -header-filter='^(?!(.*/)?stdexec-src/|(.*/)?libzip-src/|(.*/)?libunicode-src/).*$' -source-filter='^(?!.*_deps/).*$'
120134

121135
# }}}
122136
# {{{ Windows

src/Lightweight/QueryFormatter/SQLiteFormatter.hpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -498,23 +498,27 @@ class SQLiteQueryFormatter: public SqlQueryFormatter
498498
// commas so the split-on-',' parse on the runtime side is unambiguous.
499499
auto const joinComma = [](std::vector<std::string> const& v) {
500500
std::string out;
501-
for (size_t i = 0; i < v.size(); ++i)
501+
bool first = true;
502+
for (auto const& s: v)
502503
{
503-
if (i != 0)
504+
if (!first)
504505
out += ',';
505-
out += v[i];
506+
out += s;
507+
first = false;
506508
}
507509
return out;
508510
};
509511
auto const joinQuoted = [](std::vector<std::string> const& v) {
510512
std::string out;
511-
for (size_t i = 0; i < v.size(); ++i)
513+
bool first = true;
514+
for (auto const& s: v)
512515
{
513-
if (i != 0)
516+
if (!first)
514517
out += ", ";
515518
out += '"';
516-
out += v[i];
519+
out += s;
517520
out += '"';
521+
first = false;
518522
}
519523
return out;
520524
};

src/Lightweight/SqlDataDiff.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,13 @@ namespace
138138
: std::format("{}.{}", Quote(tableOnThisSide.schema, server), Quote(tableOnThisSide.name, server));
139139

140140
auto cols = std::string {};
141-
for (auto const& [i, name]: std::views::enumerate(layout.orderedColumnNames))
141+
bool firstCol = true;
142+
for (auto const& name: layout.orderedColumnNames)
142143
{
143-
if (i != 0)
144+
if (!firstCol)
144145
cols += ", ";
145146
cols += Quote(name, server);
147+
firstCol = false;
146148
}
147149

148150
auto orderBy = std::string {};

src/Lightweight/SqlDataDiff.hpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,30 @@ namespace SqlSchema
2424
/// One row-level data difference between two databases.
2525
struct RowDiff
2626
{
27+
/// Row-level classification: only-in-A, only-in-B, or changed.
2728
DiffKind kind {};
2829

2930
/// String form of the row's primary key tuple. Empty for tables without a PK
30-
/// (in which case @ref TableDataDiff::skipReason is set instead).
31+
/// (in which case `TableDataDiff::skipReason` is set instead).
3132
std::vector<std::string> primaryKey {};
3233

33-
/// For @ref DiffKind::Changed: per-column tuple of (column name, value-on-A, value-on-B)
34+
/// For `DiffKind::Changed`: per-column tuple of (column name, value-on-A, value-on-B)
3435
/// where the values differ. Empty for OnlyInA / OnlyInB.
3536
std::vector<std::tuple<std::string, std::string, std::string>> changedCells {};
3637
};
3738

3839
/// Result of comparing the rows of one table across two databases.
3940
struct TableDataDiff
4041
{
42+
/// Table name (schema-unqualified) that was compared.
4143
std::string tableName {};
4244

4345
/// All row-level diffs found, in primary-key order.
4446
std::vector<RowDiff> rows {};
4547

46-
/// Total rows scanned from each side (informational; useful for headers).
48+
/// Total rows scanned from the left-hand side (informational; useful for headers).
4749
std::size_t aRowCount = 0;
50+
/// Total rows scanned from the right-hand side (informational; useful for headers).
4851
std::size_t bRowCount = 0;
4952

5053
/// True when scanning was cut short by the @c maxRows cap.
@@ -58,8 +61,11 @@ namespace SqlSchema
5861
/// Live progress event reported during a data diff. Fired ~2 Hz at most.
5962
struct DiffProgressEvent
6063
{
64+
/// Name of the table currently being scanned.
6165
std::string tableName;
66+
/// Rows scanned so far from the left-hand side.
6267
std::size_t rowsScannedA = 0;
68+
/// Rows scanned so far from the right-hand side.
6369
std::size_t rowsScannedB = 0;
6470
std::size_t expectedRowsA = 0; ///< 0 if unknown.
6571
std::size_t expectedRowsB = 0; ///< 0 if unknown.

src/Lightweight/SqlMigration.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -817,13 +817,15 @@ namespace
817817
{
818818
auto const joinQuoted = [](std::vector<std::string> const& v) {
819819
std::string out;
820-
for (size_t i = 0; i < v.size(); ++i)
820+
bool first = true;
821+
for (auto const& s: v)
821822
{
822-
if (i != 0)
823+
if (!first)
823824
out += ", ";
824825
out += '"';
825-
out += v[i];
826+
out += s;
826827
out += '"';
828+
first = false;
827829
}
828830
return out;
829831
};

src/Lightweight/SqlQueryFormatter.hpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,10 @@ class [[nodiscard]] LIGHTWEIGHT_API SqlQueryFormatter
181181
/// @brief Whether the dialect must rebuild a table to add or drop a foreign-key
182182
/// constraint (i.e. cannot express it via `ALTER TABLE … ADD/DROP CONSTRAINT`).
183183
///
184-
/// SQLite returns `true`; every other backend defaults to `false`. The migration
185-
/// executor consults this to decide whether to take the table-rebuild path on
186-
/// `AlterTable` steps that touch foreign keys.
184+
/// Defaults to `false`; SQLiteQueryFormatter overrides to `true`. Because PG and
185+
/// MSSQL formatters inherit from SQLiteQueryFormatter, they re-override back to
186+
/// `false`. The migration executor consults this to decide whether to take the
187+
/// table-rebuild path on `AlterTable` steps that touch foreign keys.
187188
[[nodiscard]] virtual bool RequiresTableRebuildForForeignKeyChange() const noexcept
188189
{
189190
return false;

src/Lightweight/SqlSchemaDiff.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,11 +236,13 @@ namespace
236236
// Order matters for composite PKs — report a single line with both sides.
237237
auto join = [](std::vector<std::string> const& v) {
238238
auto s = std::string {};
239-
for (auto const& [i, name]: std::views::enumerate(v))
239+
bool first = true;
240+
for (auto const& name: v)
240241
{
241-
if (i != 0)
242+
if (!first)
242243
s += ", ";
243244
s += name;
245+
first = false;
244246
}
245247
return s.empty() ? std::string { "(none)" } : s;
246248
};
@@ -256,11 +258,13 @@ namespace
256258
{
257259
auto join = [](std::vector<std::string> const& v) {
258260
auto s = std::string {};
259-
for (auto const& [i, name]: std::views::enumerate(v))
261+
bool first = true;
262+
for (auto const& name: v)
260263
{
261-
if (i != 0)
264+
if (!first)
262265
s += ",";
263266
s += name;
267+
first = false;
264268
}
265269
return s;
266270
};
@@ -294,11 +298,13 @@ namespace
294298
[[nodiscard]] std::string FormatIndex(IndexDefinition const& idx)
295299
{
296300
auto cols = std::string {};
297-
for (auto const& [i, name]: std::views::enumerate(idx.columns))
301+
bool first = true;
302+
for (auto const& name: idx.columns)
298303
{
299-
if (i != 0)
304+
if (!first)
300305
cols += ",";
301306
cols += name;
307+
first = false;
302308
}
303309
return std::format("{}{}({})", idx.isUnique ? "UNIQUE " : "", idx.name, cols);
304310
}

src/Lightweight/SqlSchemaDiff.hpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,21 @@ enum class DiffKind : std::uint8_t
2323
/// A diff entry for a single column.
2424
struct ColumnDiff
2525
{
26+
/// Column name (case sensitivity follows the engines' rules — they're paired by name).
2627
std::string name;
28+
/// Column-level classification: only-in-A, only-in-B, or changed.
2729
DiffKind kind {};
2830

29-
/// For @ref DiffKind::Changed: human-readable list of differing fields
31+
/// For `DiffKind::Changed`: human-readable list of differing fields
3032
/// (e.g. `"type"`, `"nullable"`, `"defaultValue"`). Empty otherwise.
3133
std::vector<std::string> changedFields {};
3234

33-
/// Pointer into the input @ref TableList for the left-hand column. Null when
34-
/// @ref kind is @ref DiffKind::OnlyInB.
35+
/// Pointer into the input `TableList` for the left-hand column. Null when
36+
/// `kind` is `DiffKind::OnlyInB`.
3537
Column const* a = nullptr;
3638

37-
/// Pointer into the input @ref TableList for the right-hand column. Null when
38-
/// @ref kind is @ref DiffKind::OnlyInA.
39+
/// Pointer into the input `TableList` for the right-hand column. Null when
40+
/// `kind` is `DiffKind::OnlyInA`.
3941
Column const* b = nullptr;
4042
};
4143

@@ -47,20 +49,20 @@ struct TableDiff
4749
/// for identity, so the same logical table compares as equal across SQL dialects.
4850
std::string name;
4951

50-
/// Schema name on the left-hand side. Empty when @ref kind is @ref DiffKind::OnlyInB
52+
/// Schema name on the left-hand side. Empty when `kind` is `DiffKind::OnlyInB`
5153
/// or when the left-hand engine reports no schema (e.g. SQLite).
5254
std::string schemaA;
5355

54-
/// Schema name on the right-hand side. Empty when @ref kind is @ref DiffKind::OnlyInA
56+
/// Schema name on the right-hand side. Empty when `kind` is `DiffKind::OnlyInA`
5557
/// or when the right-hand engine reports no schema.
5658
std::string schemaB;
5759

58-
/// Table-level classification: @ref DiffKind::OnlyInA / @ref DiffKind::OnlyInB
59-
/// when one side lacks the table entirely; @ref DiffKind::Changed when both
60+
/// Table-level classification: `DiffKind::OnlyInA` / `DiffKind::OnlyInB`
61+
/// when one side lacks the table entirely; `DiffKind::Changed` when both
6062
/// sides have it but column / key / index definitions differ.
6163
DiffKind kind {};
6264

63-
/// Per-column diffs. Populated only when @ref kind is @ref DiffKind::Changed.
65+
/// Per-column diffs. Populated only when `kind` is `DiffKind::Changed`.
6466
std::vector<ColumnDiff> columns {};
6567

6668
/// Human-readable PK differences (e.g. `"primary key columns differ"`).
@@ -90,9 +92,9 @@ struct SchemaDiff
9092
///
9193
/// Pairs tables by **name only** so the same logical table matches across SQL dialects
9294
/// even when engine-specific schema labels differ (`dbo` vs `public` vs `""`). Both
93-
/// schema labels are kept on @ref TableDiff for the renderer. Columns are paired by name.
95+
/// schema labels are kept on `TableDiff` for the renderer. Columns are paired by name.
9496
///
95-
/// Column comparison uses the canonical @ref Column::type variant (engine-agnostic),
97+
/// Column comparison uses the canonical `Column::type` variant (engine-agnostic),
9698
/// not the dialect-dependent type string, so an `INTEGER` column compares equal whether
9799
/// the driver reports `int4`, `int`, or `INTEGER`. Other compared fields: nullability,
98100
/// size, decimal digits, default value, auto-increment, primary-key membership,

src/Lightweight/tui/MarkdownInline.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ struct InlineCodeSpan
4040
}
4141

4242
/// @brief Finds the end of a CommonMark inline code span starting at pos.
43-
[[nodiscard]] constexpr auto findInlineCodeEnd(std::string_view text, std::size_t pos)
44-
-> std::optional<InlineCodeSpan>
43+
[[nodiscard]] constexpr auto findInlineCodeEnd(std::string_view text, std::size_t pos) -> std::optional<InlineCodeSpan>
4544
{
4645
auto const tickStart = pos;
4746
while (pos < text.size() && text[pos] == '`')

src/Lightweight/tui/MarkdownTable.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ auto wrapText(std::string_view text, int maxWidth) -> std::vector<std::string>
288288
break;
289289

290290
auto const wordEnd = text.find(' ', wordStart);
291-
auto const word = text.substr(
292-
wordStart, wordEnd == std::string_view::npos ? std::string_view::npos : wordEnd - wordStart);
291+
auto const word =
292+
text.substr(wordStart, wordEnd == std::string_view::npos ? std::string_view::npos : wordEnd - wordStart);
293293
auto const wordWidth = inlineDisplayWidth(word);
294294

295295
if (currentLine.empty() && wordWidth > maxWidth)
@@ -400,8 +400,7 @@ auto stripInlineMarkdown(std::string_view text) -> std::string
400400
if (text[pos] == '[')
401401
{
402402
auto const endBracket = text.find(']', pos + 1);
403-
if (endBracket != std::string_view::npos && endBracket + 1 < text.size()
404-
&& text[endBracket + 1] == '(')
403+
if (endBracket != std::string_view::npos && endBracket + 1 < text.size() && text[endBracket + 1] == '(')
405404
{
406405
auto const endParen = text.find(')', endBracket + 2);
407406
if (endParen != std::string_view::npos)

0 commit comments

Comments
 (0)