Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions STYLE_CPP.md
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ substitute for a committed test. Tests use GoogleTest + GoogleMock with these co
give far better failure messages. The accepted exception is the boolean `EXPECT_TRUE` /
`EXPECT_FALSE` (and `ASSERT_TRUE` / `ASSERT_FALSE`), which read fine on their own. Within a
single test keep one style - do not mix, say, `EXPECT_TRUE(x)` and `EXPECT_THAT(y, IsTrue())`.
- **Name matchers unqualified - never the `::testing::` prefix inline.** Bring each matcher in with
a `using ::testing::Foo;` (or `using ::mbo::testing::Foo;`) in the test file's anonymous namespace
and use the bare name in the `EXPECT_THAT` / `ASSERT_THAT` expression; a `::testing::Foo(...)`
written inline in an assertion is the smell to fix by adding the `using`. (Fixture utilities such
as `::testing::Test` / `::testing::TempDir` are not matchers and keep their qualification.)
- **`Eq` is optional - a readability choice, not a rule.** `EXPECT_THAT(x, value)` auto-wraps a
bare value in `Eq`, so both forms compile. The value of `EXPECT_THAT` is that the line reads as
a sentence - `EXPECT_THAT(foo, Eq(25))` is "expect that foo equals 25" - so keep `Eq` where it
Expand Down
3 changes: 2 additions & 1 deletion xff/cli/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ int main(int argc, char** argv) {
[](std::string_view record) { std::cout.write(record.data(), static_cast<std::streamsize>(record.size())); },
[](std::string_view path, absl::Status status) {
std::cerr << "xff: " << path << ": " << status.message() << "\n";
});
},
style); // mode-scoped traversal defaults (modern -> sorted + parallel; find -> unordered)
return errors == 0 ? 0 : 2;
}
46 changes: 34 additions & 12 deletions xff/engine/run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <optional>
#include <string>
#include <string_view>
#include <thread>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -108,13 +109,30 @@ SymlinkMode ResolveSymlinkMode(const std::vector<std::string>& globals) {
return mode;
}

// The mode-scoped default worker count when `-j` is absent (docs/design-parallel.md
// "Parallelism control"): modern (kXff) leaves a core for the consumer and caps at
// 15 to avoid oversubscription; find/fd/rg saturate cores; an unset style stays
// sequential (the conservative in-process / test default).
std::size_t DefaultWorkers(std::optional<registry::Style> style) {
if (!style.has_value()) {
return 1;
}
const unsigned detected = std::thread::hardware_concurrency();
const std::size_t cores = detected == 0 ? 1 : detected;
if (*style == registry::Style::kXff) {
return std::max<std::size_t>(1, std::min<std::size_t>(cores - 1, 15));
}
return cores;
}

// xff --sort=none|dir|subtree|tree: per-directory sibling ordering for the walk
// (see docs/design-parallel.md). `none` keeps readdir order (find's default);
// `dir` sorts each directory's listing; `subtree` adds contiguous subtrees;
// `tree` is a total path order. Bare --sort and the legacy `name` mean `dir`.
// `tree` is a total path order. Bare --sort and the legacy `name` mean `dir`. The
// default is mode-scoped: modern (kXff) sorts each directory, find stays unordered.
// Leading global, last occurrence wins.
SortOrder ResolveSort(const std::vector<std::string>& globals) {
SortOrder sort = SortOrder::kNone;
SortOrder ResolveSort(const std::vector<std::string>& globals, std::optional<registry::Style> style) {
SortOrder sort = style == registry::Style::kXff ? SortOrder::kDir : SortOrder::kNone;
for (const std::string& global : globals) {
if (global == "--sort" || global == "--sort=dir" || global == "--sort=name") {
sort = SortOrder::kDir;
Expand All @@ -130,12 +148,11 @@ SortOrder ResolveSort(const std::vector<std::string>& globals) {
}

// xff -jN / --jobs=N: worker threads for the parallel directory read-ahead (see
// docs/design-parallel.md). `1` (the default) is the sequential walk. Leading
// global, last valid occurrence wins; a non-positive or unparseable value is
// ignored. Mode-scoped auto-defaults (e.g. modern -> all cores) arrive with the
// mode mechanism (#54); until then parallelism is explicit.
std::size_t ResolveJobs(const std::vector<std::string>& globals) {
std::size_t jobs = 1;
// docs/design-parallel.md). When absent, the count is mode-scoped (DefaultWorkers).
// Leading global, last valid occurrence wins; a non-positive or unparseable value
// is ignored.
std::size_t ResolveJobs(const std::vector<std::string>& globals, std::optional<registry::Style> style) {
std::size_t jobs = DefaultWorkers(style);
for (const std::string& global : globals) {
std::string_view value;
if (global.starts_with("--jobs=")) {
Expand Down Expand Up @@ -455,7 +472,12 @@ std::optional<std::string> UnusedCaptureName(const parser::Expr& expr, const std

} // namespace

int RunFind(const parser::Command& command, const vfs::FileSystem& fs, EmitFn emit, WalkErrorFn on_error) {
int RunFind(
const parser::Command& command,
const vfs::FileSystem& fs,
EmitFn emit,
WalkErrorFn on_error,
std::optional<registry::Style> style) {
const parser::Expr* const expression = command.expression.get();
const bool has_action = expression != nullptr && ContainsAction(*expression);
// --implicit-print=yes|no overrides find's default-print rule (otherwise !has_action).
Expand All @@ -476,8 +498,8 @@ int RunFind(const parser::Command& command, const vfs::FileSystem& fs, EmitFn em
}
WalkOptions options;
options.symlinks = ResolveSymlinkMode(command.globals);
options.sort = ResolveSort(command.globals);
options.workers = ResolveJobs(command.globals);
options.sort = ResolveSort(command.globals, style);
options.workers = ResolveJobs(command.globals, style);
const render::Format format = ResolveFormat(command.globals);
const std::optional<std::string> tmpl = ResolveTemplate(command.globals);
// A -capture whose {capture.NAME} is never referenced ran a subprocess for
Expand Down
16 changes: 15 additions & 1 deletion xff/engine/run.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@
#ifndef XFF_ENGINE_RUN_H_
#define XFF_ENGINE_RUN_H_

#include <optional>

#include "xff/engine/evaluate.h"
#include "xff/engine/walk.h"
#include "xff/parser/ast.h"
#include "xff/registry/descriptor.h"
#include "xff/vfs/filesystem.h"

namespace xff::engine {
Expand All @@ -30,7 +33,18 @@ namespace xff::engine {
//
// Returns the number of per-path errors encountered (0 == clean). The CLI maps
// a nonzero count to exit 2; the full exit-code model is a follow-up.
int RunFind(const parser::Command& command, const vfs::FileSystem& fs, EmitFn emit, WalkErrorFn on_error);
//
// `style` selects the mode-scoped traversal defaults applied when the user gives
// no `--sort` / `-j`: kXff (modern) sorts each directory (`--sort=dir`) and runs
// a capped parallel walk; kFind matches find (unordered) but saturates cores.
// `std::nullopt` keeps the conservative defaults (unordered, single-threaded) and
// is what the in-process callers/tests use; the CLI passes the active style.
int RunFind(
const parser::Command& command,
const vfs::FileSystem& fs,
EmitFn emit,
WalkErrorFn on_error,
std::optional<registry::Style> style = std::nullopt);

} // namespace xff::engine

Expand Down
34 changes: 34 additions & 0 deletions xff/engine/run_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,28 @@ struct RunTest : ::testing::Test {
return records;
}

// Runs the bare root under `style` to exercise the mode-scoped traversal
// defaults (RunFind's `style`), returning records with the terminator stripped.
std::vector<std::string> RunStyled(registry::Style style) {
const auto command = parser::Parse({root_.string()});
EXPECT_THAT(command, IsOk());
std::vector<std::string> records;
if (!command.ok()) {
return records;
}
RunFind(
*command, fs_,
[&](std::string_view record) {
std::string text(record);
if (!text.empty() && (text.back() == '\n' || text.back() == '\0')) {
text.pop_back();
}
records.push_back(std::move(text));
},
[](std::string_view, absl::Status) {}, style);
return records;
}

vfs::LocalFs fs_;
fs::path root_;
int last_errors_ = 0;
Expand All @@ -120,6 +142,18 @@ TEST_F(RunTest, NoExpressionPrintsEverything) {
EXPECT_THAT(last_errors_, 0);
}

TEST_F(RunTest, ModeScopedSortDefault) {
// With no --sort, the active style picks the default: modern (kXff) sorts each
// directory's listing, so the walk is deterministic (root, then a.txt < b.md <
// sub as a block, then sub's contents). find leaves it unordered (same set).
EXPECT_THAT(
RunStyled(registry::Style::kXff),
ElementsAre(root_.string(), Path("a.txt"), Path("b.md"), Path("sub"), Path("sub/c.txt")));
EXPECT_THAT(
RunStyled(registry::Style::kFind),
UnorderedElementsAre(root_.string(), Path("a.txt"), Path("b.md"), Path("sub"), Path("sub/c.txt")));
}

TEST_F(RunTest, SortNameVisitsSiblingsInDeterministicOrder) {
// --sort=name orders each directory's entries by name, so the whole walk is
// deterministic: root first, then a.txt < b.md < sub, then sub/c.txt. ElementsAre
Expand Down
Loading