Skip to content

Commit 5f5513f

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 7246ce0 commit 5f5513f

23 files changed

Lines changed: 334 additions & 239 deletions

docs/CMakeLists.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ message(STATUS "Doxygen found: ${DOXYGEN_EXECUTABLE}")
1414
set(DOXYGEN_BUILTIN_STL_SUPPORT YES)
1515
set(DOXYGEN_CASE_SENSE_NAMES NO)
1616
set(DOXYGEN_CLASS_DIAGRAMS NO)
17-
set(DOXYGEN_EXCLUDE bin "${PROJECT_SOURCE_DIR}/src/tests" "${PROJECT_SOURCE_DIR}/src/tools")
17+
set(DOXYGEN_EXCLUDE bin
18+
"${PROJECT_SOURCE_DIR}/src/tests"
19+
"${PROJECT_SOURCE_DIR}/src/tools"
20+
"${PROJECT_SOURCE_DIR}/src/Lightweight/tui")
1821
set(DOXYGEN_EXTRACT_ALL NO)
1922
set(DOXYGEN_EXTRACT_LOCAL_CLASSES NO)
2023
set(DOXYGEN_FILE_PATTERNS *.hpp)

src/Lightweight/Config/ProfileStore.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ SqlConnectInfo Profile::ToConnectInfo(std::string_view password) const
4444
std::ranges::transform(s, s.begin(), [](unsigned char c) { return static_cast<char>(std::tolower(c)); });
4545
return s;
4646
}();
47-
if (lower.find("pwd=") != std::string::npos || lower.find("password=") != std::string::npos)
47+
if (lower.contains("pwd=") || lower.contains("password="))
4848
return SqlConnectionString { connectionString };
4949

5050
std::string extended = connectionString;

src/Lightweight/Config/ProfileStore.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ struct Profile
8686
[[nodiscard]] LIGHTWEIGHT_API SqlConnectInfo ToConnectInfo(std::string_view password = {}) const;
8787
};
8888

89+
#if defined(_MSC_VER)
90+
#pragma warning(push)
91+
#pragma warning(disable : 4251) // STL types in DLL interface
92+
#endif
93+
8994
/// In-memory collection of named `Profile`s plus "which is the default".
9095
class LIGHTWEIGHT_API ProfileStore
9196
{
@@ -156,4 +161,8 @@ class LIGHTWEIGHT_API ProfileStore
156161
std::string _defaultProfile;
157162
};
158163

164+
#if defined(_MSC_VER)
165+
#pragma warning(pop)
166+
#endif
167+
159168
} // namespace Lightweight::Config

src/Lightweight/QueryFormatter/SQLiteFormatter.hpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ class SQLiteQueryFormatter: public SqlQueryFormatter
2525
}
2626

2727
public:
28+
/// SQLite cannot express `ALTER TABLE … ADD/DROP CONSTRAINT`, so any FK change
29+
/// goes through the table-rebuild path; PG and MSSQL override this back to
30+
/// `false` because they inherit from SQLiteQueryFormatter but do support FK ALTER.
31+
[[nodiscard]] bool RequiresTableRebuildForForeignKeyChange() const noexcept override
32+
{
33+
return true;
34+
}
35+
2836
/// Builds the SQL query used to check whether a column exists on a SQLite table.
2937
///
3038
/// The migration executor uses this to resolve the `-- LIGHTWEIGHT_SQLITE_GUARD:`
@@ -486,23 +494,27 @@ class SQLiteQueryFormatter: public SqlQueryFormatter
486494
// commas so the split-on-',' parse on the runtime side is unambiguous.
487495
auto const joinComma = [](std::vector<std::string> const& v) {
488496
std::string out;
489-
for (auto const& [i, s]: std::views::enumerate(v))
497+
bool first = true;
498+
for (auto const& s: v)
490499
{
491-
if (i != 0)
500+
if (!first)
492501
out += ',';
493502
out += s;
503+
first = false;
494504
}
495505
return out;
496506
};
497507
auto const joinQuoted = [](std::vector<std::string> const& v) {
498508
std::string out;
499-
for (auto const& [i, s]: std::views::enumerate(v))
509+
bool first = true;
510+
for (auto const& s: v)
500511
{
501-
if (i != 0)
512+
if (!first)
502513
out += ", ";
503514
out += '"';
504515
out += s;
505516
out += '"';
517+
first = false;
506518
}
507519
return out;
508520
};

src/Lightweight/Secrets/ISecretBackend.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace Lightweight::Secrets
2727
/// Bare references (no prefix) walk the backend chain registered with
2828
/// `SecretResolver::RegisterBackend()`; the first backend that returns a
2929
/// non-empty result wins.
30-
class ISecretBackend
30+
class LIGHTWEIGHT_API ISecretBackend
3131
{
3232
public:
3333
ISecretBackend() = default;

src/Lightweight/Secrets/SecretResolver.cpp

Lines changed: 50 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// SPDX-License-Identifier: Apache-2.0
22

33
#include "SecretResolver.hpp"
4-
54
#include "backends/EnvBackend.hpp"
65
#include "backends/FileBackend.hpp"
76
#include "backends/StdinBackend.hpp"
@@ -10,74 +9,77 @@
109
#include <cstdlib>
1110
#include <filesystem>
1211
#include <format>
12+
#include <ranges>
1313

1414
namespace Lightweight::Secrets
1515
{
1616

1717
namespace
1818
{
1919

20-
/// Splits `secretRef` into `{prefix, key}` where the prefix is everything
21-
/// before the first colon. Bare references (no colon) yield an empty prefix
22-
/// and the whole string as the key.
23-
struct SecretRefParts
24-
{
25-
std::string_view prefix;
26-
std::string_view key;
27-
};
20+
/// Splits `secretRef` into `{prefix, key}` where the prefix is everything
21+
/// before the first colon. Bare references (no colon) yield an empty prefix
22+
/// and the whole string as the key.
23+
struct SecretRefParts
24+
{
25+
std::string_view prefix;
26+
std::string_view key;
27+
};
2828

29-
SecretRefParts SplitSecretRef(std::string_view secretRef) noexcept
30-
{
31-
auto const colon = secretRef.find(':');
32-
if (colon == std::string_view::npos)
33-
return { .prefix = {}, .key = secretRef };
34-
return { .prefix = secretRef.substr(0, colon), .key = secretRef.substr(colon + 1) };
35-
}
29+
SecretRefParts SplitSecretRef(std::string_view secretRef) noexcept
30+
{
31+
auto const colon = secretRef.find(':');
32+
if (colon == std::string_view::npos)
33+
return { .prefix = {}, .key = secretRef };
34+
return { .prefix = secretRef.substr(0, colon), .key = secretRef.substr(colon + 1) };
35+
}
3636

37-
/// MSVC treats `std::getenv` as deprecated in favour of `_dupenv_s`; we wrap
38-
/// it once here so the suppression is centralised rather than scattered at
39-
/// every call site. The one-read-at-start-up use is safe enough — we never
40-
/// interleave reads with `putenv` on the same variable.
41-
char const* SafeGetenv(char const* name) noexcept
42-
{
37+
/// MSVC treats `std::getenv` as deprecated in favour of `_dupenv_s`; we wrap
38+
/// it once here so the suppression is centralised rather than scattered at
39+
/// every call site. The one-read-at-start-up use is safe enough — we never
40+
/// interleave reads with `putenv` on the same variable.
41+
char const* SafeGetenv(char const* name) noexcept
42+
{
4343
#ifdef _WIN32
4444
#pragma warning(push)
4545
#pragma warning(disable : 4996)
4646
#endif
47-
return std::getenv(name);
47+
return std::getenv(name);
4848
#ifdef _WIN32
4949
#pragma warning(pop)
5050
#endif
51-
}
51+
}
5252

53-
/// Resolves `~` / `$HOME` in the default credentials path, matching the
54-
/// behaviour users expect from shells and pg_hba.conf-style configs.
55-
std::filesystem::path DefaultCredentialsPath()
56-
{
53+
/// Resolves `~` / `$HOME` in the default credentials path, matching the
54+
/// behaviour users expect from shells and pg_hba.conf-style configs.
55+
std::filesystem::path DefaultCredentialsPath()
56+
{
5757
#ifdef _WIN32
58-
if (char const* appData = SafeGetenv("APPDATA"); appData && *appData)
59-
return std::filesystem::path(appData) / "dbtool" / "credentials";
60-
return "dbtool-credentials";
58+
if (char const* appData = SafeGetenv("APPDATA"); appData && *appData)
59+
return std::filesystem::path(appData) / "dbtool" / "credentials";
60+
return "dbtool-credentials";
6161
#else
62-
if (char const* xdg = SafeGetenv("XDG_CONFIG_HOME"); xdg && *xdg)
63-
return std::filesystem::path(xdg) / "Lightweight" / "credentials";
64-
if (char const* home = SafeGetenv("HOME"); home && *home)
65-
return std::filesystem::path(home) / ".config" / "Lightweight" / "credentials";
66-
return "Lightweight-credentials";
62+
if (char const* xdg = SafeGetenv("XDG_CONFIG_HOME"); xdg && *xdg)
63+
return std::filesystem::path(xdg) / "Lightweight" / "credentials";
64+
if (char const* home = SafeGetenv("HOME"); home && *home)
65+
return std::filesystem::path(home) / ".config" / "Lightweight" / "credentials";
66+
return "Lightweight-credentials";
6767
#endif
68-
}
68+
}
6969

70-
std::string JoinBackendNames(std::vector<std::string> const& names)
71-
{
72-
std::string joined;
73-
for (size_t i = 0; i < names.size(); ++i)
70+
std::string JoinBackendNames(std::vector<std::string> const& names)
7471
{
75-
if (i != 0)
76-
joined += ", ";
77-
joined += names[i];
72+
std::string joined;
73+
bool first = true;
74+
for (auto const& name: names)
75+
{
76+
if (!first)
77+
joined += ", ";
78+
joined += name;
79+
first = false;
80+
}
81+
return joined;
7882
}
79-
return joined;
80-
}
8183

8284
} // namespace
8385

@@ -114,7 +116,7 @@ std::expected<std::string, ResolveError> SecretResolver::ResolveExplicit(std::st
114116
}
115117

116118
std::expected<std::string, ResolveError> SecretResolver::ResolveBare(std::string_view secretRef,
117-
std::string_view profileName) const
119+
std::string_view profileName) const
118120
{
119121
// Bare ref: walk the registered chain in order. Each backend gets a key
120122
// shaped to its own conventions so callers don't have to remember whether
@@ -143,7 +145,7 @@ std::expected<std::string, ResolveError> SecretResolver::ResolveBare(std::string
143145
}
144146

145147
std::expected<std::string, ResolveError> SecretResolver::Resolve(std::string_view secretRef,
146-
std::string_view profileName) const
148+
std::string_view profileName) const
147149
{
148150
if (secretRef.empty())
149151
return std::unexpected(ResolveError {

src/Lightweight/Secrets/SecretResolver.hpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ struct ResolveError
3737
std::string message;
3838
};
3939

40+
#if defined(_MSC_VER)
41+
#pragma warning(push)
42+
#pragma warning(disable : 4251) // STL types in DLL interface
43+
#endif
44+
4045
/// Central lookup for opaque `secretRef` strings. Owns a list of registered
4146
/// backends and dispatches based on either an explicit `prefix:` or
4247
/// registration order for bare references.
@@ -46,8 +51,10 @@ class LIGHTWEIGHT_API SecretResolver
4651
SecretResolver() = default;
4752
~SecretResolver() = default;
4853
SecretResolver(SecretResolver const&) = delete;
54+
/// Move-construct: defaulted; preserves backend registration order.
4955
SecretResolver(SecretResolver&&) = default;
5056
SecretResolver& operator=(SecretResolver const&) = delete;
57+
/// Move-assign: defaulted; preserves backend registration order.
5158
SecretResolver& operator=(SecretResolver&&) = default;
5259

5360
/// Appends a backend to the chain. The backend's `Name()` is used both as
@@ -73,7 +80,7 @@ class LIGHTWEIGHT_API SecretResolver
7380
/// registered but declines to build at runtime (e.g. `secretservice:` on
7481
/// a headless box), or the bare-ref chain produced no hits.
7582
[[nodiscard]] std::expected<std::string, ResolveError> Resolve(std::string_view secretRef,
76-
std::string_view profileName) const;
83+
std::string_view profileName) const;
7784

7885
/// Returns the registered backend prefixes in registration order, for
7986
/// diagnostic / debugging output.
@@ -83,15 +90,19 @@ class LIGHTWEIGHT_API SecretResolver
8390
[[nodiscard]] ISecretBackend* Lookup(std::string_view name) const noexcept;
8491

8592
[[nodiscard]] std::expected<std::string, ResolveError> ResolveExplicit(std::string_view prefix,
86-
std::string_view key,
87-
std::string_view profileName) const;
93+
std::string_view key,
94+
std::string_view profileName) const;
8895

8996
[[nodiscard]] std::expected<std::string, ResolveError> ResolveBare(std::string_view secretRef,
90-
std::string_view profileName) const;
97+
std::string_view profileName) const;
9198

9299
std::vector<std::shared_ptr<ISecretBackend>> _backends;
93100
};
94101

102+
#if defined(_MSC_VER)
103+
#pragma warning(pop)
104+
#endif
105+
95106
/// Convenience: builds a default resolver wired with the platform-neutral
96107
/// backends (`env:`, `file:`, `stdin:`) suitable for every build of
97108
/// Lightweight, regardless of Qt / libsecret availability.

0 commit comments

Comments
 (0)