From 85163729a9ee14fafbfba995f2bf96363d8d4aee Mon Sep 17 00:00:00 2001 From: helly25 <6420169+helly25@users.noreply.github.com> Date: Tue, 7 Jul 2026 00:06:28 +0100 Subject: [PATCH] feat(fields): add the {lines} first-class line-count field (#114) A per-text-file line count in the field vocabulary: {lines}, %{lines} in -printf, and --template - wc -l style but also counting a final unterminated line. Empty for a non-regular, unreadable, or binary file (grep/ripgrep's NUL-byte-in-first-8KiB heuristic), so binaries render nothing rather than a misleading count. - xff/content: CountLines (pure, reuses the CollectLineMatches grep line semantics via ForEachLine) + FileLineCount (reads via mbo::file::Artefact, skips binary), next to the line-match logic they share. - xff/fields: LinesField reads by path (like {hash}), gated on a regular-file metadata type; registered in the field table + FieldDocs (so --help=fields, man, and markdown list it by construction). - Tests: CountLines/FileLineCount unit tests, LinesField render tests (text/no-newline/empty/binary/unreadable/non-regular), and a CLI bashtest driving -printf %{lines} end to end. The aggregate side (sum + distribution across matches) is the histograms work (#81), not part of this field. --- TODO.md | 12 +++---- xff/cli/BUILD.bazel | 9 ++++++ xff/cli/lines_test.sh | 59 ++++++++++++++++++++++++++++++++++ xff/content/BUILD.bazel | 2 ++ xff/content/line_match.cc | 22 +++++++++++++ xff/content/line_match.h | 13 ++++++++ xff/content/line_match_test.cc | 39 ++++++++++++++++++++++ xff/fields/BUILD.bazel | 1 + xff/fields/fields.cc | 20 ++++++++++++ xff/fields/fields_test.cc | 27 ++++++++++++++++ 10 files changed, 198 insertions(+), 6 deletions(-) create mode 100755 xff/cli/lines_test.sh diff --git a/TODO.md b/TODO.md index 0dc3b24..98b206e 100644 --- a/TODO.md +++ b/TODO.md @@ -188,12 +188,12 @@ remains below is the design-forked / larger work. conventional precedence `NOT > AND/-nand > XOR/-xnor > OR/-nor`; the strict find style rejects them. (`-xor` matches exactly one side; the rest are the negations of and/or/xor.) -- **Line count as a first-class metric** (2026-07-04): a fully-featured per-text-file line - count, beyond the shipped grep-style `-grep -c` / `--count` (#92). It should be a field in - the vocabulary (`{lines}`, usable in `-printf` `%{lines}` / `--format` / per-file output), - a `--summary` value (sum + a distribution / histogram of line counts across matches), and - available to final / aggregate outputs - count lines everywhere counts and sizes already - appear. (Binary files: no count, like the content detector.) +- **Line count as a first-class metric** (2026-07-04): **the `{lines}` field shipped** - a + per-text-file line count in the field vocabulary (`{lines}`, `-printf` `%{lines}`, `--template`), + `wc -l`-style but also counting a final unterminated line; empty for a binary / unreadable / + non-regular file (`content::FileLineCount` + `CountLines`, reusing the grep NUL-byte binary + heuristic). **Remaining:** surfacing it as an aggregate (sum + a distribution across matches), + which is the `lines` metric of the histograms work (#81), not a separate item. - **Hash-verification workflow (#109) - [DISCUSS].** The hashing primitives shipped (#105: `xff/hash` + the `{hash}` / `{hash:sha256}` field + the `-hash` action + hex/base64 via `mbo::digest`). Still to design and build: read an expected hash into a variable, an diff --git a/xff/cli/BUILD.bazel b/xff/cli/BUILD.bazel index 89547ce..0e936f7 100644 --- a/xff/cli/BUILD.bazel +++ b/xff/cli/BUILD.bazel @@ -314,6 +314,15 @@ xff_golden_cases( setup = "testdata/walk/setup.sh", ) +# End-to-end test of the {lines} per-file line-count field via the %{lines} -printf escape: +# text-file counts, no-trailing-newline, empty (0), binary (empty), and non-regular (empty). +bashtest( + name = "lines_test", + size = "small", + srcs = ["lines_test.sh"], + data = [":xff"], +) + # End-to-end test of -ls column alignment (via the ColumnBuffer) and the --buffer # modes (auto/off/all/N). bashtest( diff --git a/xff/cli/lines_test.sh b/xff/cli/lines_test.sh new file mode 100755 index 0000000..57eb4be --- /dev/null +++ b/xff/cli/lines_test.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) The helly25 authors (helly25.com) +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# End-to-end test of the {lines} field (per-file text line count) via the %{lines} -printf +# escape: a multi-line file, a file with no trailing newline, an empty file (0), the empty +# render for a binary file (a NUL byte), and the empty render for a non-regular entry. Drives +# the real binary (reads files). + +set -euo pipefail + +# shellcheck disable=SC1090,SC1091,SC2154 +source "${helly25_bashtest}" + +_xff_bin() { + local bin="${TEST_SRCDIR}/${TEST_WORKSPACE}/xff/cli/xff" + if [[ ! -x "${bin}" ]]; then + bin="$(find "${TEST_SRCDIR}" -type f -name xff -path '*xff/cli/xff' 2>/dev/null | head -1)" + fi + echo "${bin}" +} + +test::lines_field_counts_text_files() { + local dir out + dir="${TEST_TMPDIR}/lines" + mkdir -p "${dir}" + printf 'a\nb\nc\n' >"${dir}/three.txt" # 3 lines + printf 'x' >"${dir}/one.txt" # 1 line, no trailing newline + printf '' >"${dir}/empty.txt" # 0 lines + printf 'a\0b\n' >"${dir}/bin.dat" # binary (NUL byte) -> empty + out="$(XFF_CONFIG="${TEST_TMPDIR}/none" "$(_xff_bin)" "${dir}" -type f -printf '%{lines}|%{name}\n' 2>&1)" + expect_output_contains "3|three.txt" "${out}" + expect_output_contains "1|one.txt" "${out}" + expect_output_contains "0|empty.txt" "${out}" + expect_output_contains "|bin.dat" "${out}" # a binary file has no line count +} + +test::lines_field_empty_for_non_regular() { + local dir out + dir="${TEST_TMPDIR}/linesdir" + mkdir -p "${dir}" + # The directory itself is non-regular, so {lines} renders empty (bracketed to show it). + out="$(XFF_CONFIG="${TEST_TMPDIR}/none" "$(_xff_bin)" "${dir}" -maxdepth 0 -type d -printf '[%{lines}]\n' 2>&1)" + expect_output_contains "[]" "${out}" +} + +test_runner diff --git a/xff/content/BUILD.bazel b/xff/content/BUILD.bazel index dfebf7d..8b442eb 100644 --- a/xff/content/BUILD.bazel +++ b/xff/content/BUILD.bazel @@ -24,6 +24,8 @@ cc_library( visibility = ["//xff:__subpackages__"], deps = [ "@abseil-cpp//absl/functional:function_ref", + "@abseil-cpp//absl/status:statusor", + "@helly25_mbo//mbo/file:artefact_cc", ], ) diff --git a/xff/content/line_match.cc b/xff/content/line_match.cc index 051d746..d50830c 100644 --- a/xff/content/line_match.cc +++ b/xff/content/line_match.cc @@ -17,10 +17,13 @@ #include #include +#include #include #include #include "absl/functional/function_ref.h" +#include "absl/status/statusor.h" +#include "mbo/file/artefact.h" namespace xff::content { namespace { @@ -106,4 +109,23 @@ std::vector CollectLineMatchesWithContext( return result; } +std::size_t CountLines(std::string_view content) { + std::size_t lines = 0; + ForEachLine(content, [&lines](std::size_t, std::string_view) { ++lines; }); + return lines; +} + +std::optional FileLineCount(std::string_view path) { + const absl::StatusOr artefact = mbo::file::Artefact::Read(path); + if (!artefact.ok()) { + return std::nullopt; // unreadable / missing -> nothing to count + } + const std::string_view content = artefact->data; + constexpr std::size_t kBinarySniffBytes = std::size_t{8} * 1'024; + if (content.substr(0, std::min(content.size(), kBinarySniffBytes)).find('\0') != std::string_view::npos) { + return std::nullopt; // a NUL in the first 8 KiB marks the file binary; skip it (like content search) + } + return CountLines(content); +} + } // namespace xff::content diff --git a/xff/content/line_match.h b/xff/content/line_match.h index faed701..8e6f5f5 100644 --- a/xff/content/line_match.h +++ b/xff/content/line_match.h @@ -17,6 +17,7 @@ #define XFF_CONTENT_LINE_MATCH_H_ #include +#include #include #include @@ -63,6 +64,18 @@ std::vector CollectLineMatchesWithContext( std::size_t before, std::size_t after); +// The number of text lines in `content`, using the same line semantics as CollectLineMatches: +// each '\n'-terminated segment is a line, plus a trailing partial line with no final '\n'. So +// "" is 0, "a\nb\n" is 2, "a\nb" is 2, "a" is 1, "\n" is 1. Like `wc -l` but also counting an +// unterminated final line. +std::size_t CountLines(std::string_view content); + +// CountLines for the regular file at `path`, or nullopt when there is nothing to count: an +// unreadable file, or a binary one (a NUL byte in the first 8 KiB, grep/ripgrep's heuristic - the +// same rule content search uses to skip binaries). Reads the whole file, so it is expensive. The +// caller is responsible for restricting this to regular files. +std::optional FileLineCount(std::string_view path); + } // namespace xff::content #endif // XFF_CONTENT_LINE_MATCH_H_ diff --git a/xff/content/line_match_test.cc b/xff/content/line_match_test.cc index 8027e64..069474e 100644 --- a/xff/content/line_match_test.cc +++ b/xff/content/line_match_test.cc @@ -15,6 +15,11 @@ #include "xff/content/line_match.h" +#include +#include +#include +#include +#include #include #include "absl/strings/match.h" @@ -25,9 +30,11 @@ namespace xff::content { namespace { using ::testing::ElementsAre; +using ::testing::Eq; using ::testing::Field; using ::testing::IsEmpty; using ::testing::Matcher; +using ::testing::Optional; Matcher LineIs(std::size_t number, std::string_view text) { return AllOf(Field("number", &LineMatch::number, number), Field("text", &LineMatch::text, text)); @@ -136,5 +143,37 @@ TEST_F(LineContextTest, GapStartsANewGroup) { ElementsAre(CtxIs(1, "HIT", true, 0), CtxIs(2, "a", false, 0), CtxIs(6, "HIT", true, 1))); } +struct LineCountTest : ::testing::Test {}; + +TEST_F(LineCountTest, CountLinesMatchesGrepSemantics) { + EXPECT_THAT(CountLines(""), 0U); + EXPECT_THAT(CountLines("a"), 1U); // an unterminated single line + EXPECT_THAT(CountLines("a\n"), 1U); + EXPECT_THAT(CountLines("a\nb\n"), 2U); + EXPECT_THAT(CountLines("a\nb"), 2U); // a final line with no trailing newline still counts + EXPECT_THAT(CountLines("\n"), 1U); // a lone newline is one (empty) line + EXPECT_THAT(CountLines("\n\n"), 2U); + EXPECT_THAT(CountLines("a\r\nb\r\n"), 2U); // CRLF counts like LF +} + +TEST_F(LineCountTest, FileLineCountReadsAndCounts) { + const std::string path = std::string(::testing::TempDir()) + "/xff_line_count_txt"; + { std::ofstream(path) << "one\ntwo\nthree\n"; } + EXPECT_THAT(FileLineCount(path), Optional(Eq(std::size_t{3}))); + { std::ofstream(path) << "no trailing newline"; } + EXPECT_THAT(FileLineCount(path), Optional(Eq(std::size_t{1}))); + { std::ofstream(path) << ""; } // truncate to empty + EXPECT_THAT(FileLineCount(path), Optional(Eq(std::size_t{0}))); + std::remove(path.c_str()); +} + +TEST_F(LineCountTest, FileLineCountSkipsBinaryAndUnreadable) { + const std::string path = std::string(::testing::TempDir()) + "/xff_line_count_bin"; + { std::ofstream(path, std::ios::binary).write("a\0b\n", 4); } // a NUL byte in the content + EXPECT_THAT(FileLineCount(path), Eq(std::nullopt)); // a NUL byte marks the file binary + std::remove(path.c_str()); + EXPECT_THAT(FileLineCount(std::string(::testing::TempDir()) + "/xff_line_count_absent"), Eq(std::nullopt)); +} + } // namespace } // namespace xff::content diff --git a/xff/fields/BUILD.bazel b/xff/fields/BUILD.bazel index efea860..c0865c7 100644 --- a/xff/fields/BUILD.bazel +++ b/xff/fields/BUILD.bazel @@ -22,6 +22,7 @@ cc_library( hdrs = ["fields.h"], visibility = ["//xff:__subpackages__"], deps = [ + "//xff/content:line_match_cc", "//xff/datetime:datetime_cc", "//xff/hash:hash_cc", "//xff/language:language_cc", diff --git a/xff/fields/fields.cc b/xff/fields/fields.cc index c199fba..02728da 100644 --- a/xff/fields/fields.cc +++ b/xff/fields/fields.cc @@ -40,6 +40,7 @@ #include "absl/status/statusor.h" #include "absl/time/time.h" #include "mbo/container/limited_map.h" +#include "xff/content/line_match.h" #include "xff/datetime/datetime.h" #include "xff/hash/hash.h" #include "xff/language/language.h" @@ -271,6 +272,19 @@ std::string HashField(std::string_view, std::string_view qualifier, const Render return hash::HashFile(spec->algo, ctx.path, spec->encoding).value_or(""); } +// {lines}: the number of text lines in the entry's file content (like `wc -l`, but also counting a +// final line with no trailing newline). Empty for a non-regular, unreadable, or binary file (a NUL +// byte in the first 8 KiB, grep/ripgrep's heuristic, so binaries render nothing rather than a +// misleading count). Reads the file, so it is expensive; composes with --summary / -printf to tally +// lines across matches. +std::string LinesField(std::string_view, std::string_view, const RenderContext& ctx) { + if (ctx.metadata.type != vfs::FileType::kRegular) { + return ""; // only regular files have countable content + } + const std::optional lines = content::FileLineCount(ctx.path); + return lines.has_value() ? std::to_string(*lines) : ""; +} + // {lang} / {language}: the entry's programming/markup language (github-linguist name, e.g. "C++", // "Python"), from its filename/extension via the language table; empty when unrecognized. Content // is not read, so it is cheap; composes with --summary group-by to tally files per language. @@ -381,6 +395,7 @@ constexpr auto kFieldTable = mbo::container::MakeLimitedMap( FieldEntry{"lang", &LanguageField}, FieldEntry{"language", &LanguageField}, FieldEntry{"line", &LineField}, + FieldEntry{"lines", &LinesField}, FieldEntry{"links", &LinksField}, FieldEntry{"match", &MatchField}, FieldEntry{"mode", &ModeField}, @@ -774,6 +789,11 @@ std::vector FieldDocs() { .group = "content", .header = "Content", .summary = "file digest; {hash:ALGO[/ENCODING]} picks the algorithm (default sha256) and hex/base64"}, + {.name = "lines", + .aliases = {}, + .group = "content", + .header = "Content", + .summary = "text line count (empty for a binary/unreadable file); reads the file"}, // Owner & mode. {.name = "user", .aliases = {}, .group = "owner", .header = "Owner & mode", .summary = "owner user name"}, {.name = "group", .aliases = {}, .group = "owner", .header = "Owner & mode", .summary = "owner group name"}, diff --git a/xff/fields/fields_test.cc b/xff/fields/fields_test.cc index d6b39d5..89bc495 100644 --- a/xff/fields/fields_test.cc +++ b/xff/fields/fields_test.cc @@ -147,6 +147,33 @@ TEST_F(FieldsTest, HashFieldOfUnreadableFileIsEmpty) { EXPECT_THAT(Template::Compile("[{hash}]").Render(RenderContext{.path = absent, .metadata = md}), "[]"); } +TEST_F(FieldsTest, LinesFieldCountsTextLines) { + const vfs::Metadata md = Meta(vfs::FileType::kRegular, 0); + const std::string path = std::string(::testing::TempDir()) + "/xff_fields_lines"; + { std::ofstream(path) << "one\ntwo\nthree\n"; } + EXPECT_THAT(Template::Compile("{lines}").Render(RenderContext{.path = path, .metadata = md}), "3"); + { std::ofstream(path) << "no trailing newline"; } // a final unterminated line still counts + EXPECT_THAT(Template::Compile("{lines}").Render(RenderContext{.path = path, .metadata = md}), "1"); + { std::ofstream(path) << ""; } // truncate: an empty file is zero lines + EXPECT_THAT(Template::Compile("{lines}").Render(RenderContext{.path = path, .metadata = md}), "0"); + std::remove(path.c_str()); +} + +TEST_F(FieldsTest, LinesFieldIsEmptyForBinaryUnreadableOrNonRegular) { + const vfs::Metadata reg = Meta(vfs::FileType::kRegular, 0); + const std::string path = std::string(::testing::TempDir()) + "/xff_fields_lines_bin"; + { std::ofstream(path, std::ios::binary).write("a\0b\n", 4); } // a NUL byte in the content + // A NUL byte marks the file binary -> empty, like the content detector. + EXPECT_THAT(Template::Compile("[{lines}]").Render(RenderContext{.path = path, .metadata = reg}), "[]"); + // A regular file that cannot be read -> empty. + const std::string absent = std::string(::testing::TempDir()) + "/xff_fields_lines_absent"; + EXPECT_THAT(Template::Compile("[{lines}]").Render(RenderContext{.path = absent, .metadata = reg}), "[]"); + // A non-regular entry is never counted, even at a readable path. + const vfs::Metadata dir = Meta(vfs::FileType::kDirectory, 0); + EXPECT_THAT(Template::Compile("[{lines}]").Render(RenderContext{.path = path, .metadata = dir}), "[]"); + std::remove(path.c_str()); +} + TEST_F(FieldsTest, TimeFieldQualifiers) { vfs::Metadata md = Meta(vfs::FileType::kRegular, 0); md.mtime = absl::FromUnixSeconds(1'700'000'000); // 2023-11-14, mid-month: the year is timezone-stable