ci: adopt pre-commit (clang-format/shfmt/shellcheck/actionlint + policy hooks) - #122
Merged
Conversation
…cy hooks) Adopt helly25/mbo's pre-commit setup as a CI element: - .pre-commit-config.yaml: generic hygiene (large-files, merge-conflict, check-yaml, end-of-file-fixer, trailing-whitespace), actionlint, shfmt, shellcheck, clang-format (pinned v20.1.8 to match the hermetic toolchain byte-for-byte; verified zero reformatting), and local repo-policy pygrep hooks (no-do-not-merge, no-em-dashes, no-todos-without-context). Drops mbo repo-specific hooks (mope/bazelmod/version) that do not apply to xff. - main.yml: new `pre-commit` job (mirrors mbo), wired into the `done` gate. Removed the standalone no-em-dash grep step; the pre-commit no-em-dashes hook owns it now. The hermetic clang-format test step stays as the build-matched authority and a drift detector against the pre-commit pin. - explain_test.sh: shfmt normalized one inline-comment spacing. clang-tidy and the heavier linters (markdownlint/prettier/yamllint/ buildifier) follow via trunk. Validated locally: `pre-commit run -a` all green; bazel test //xff/cli/... passes.
helly25
enabled auto-merge (squash)
June 24, 2026 22:50
helly25
added a commit
that referenced
this pull request
Jul 10, 2026
Record the follow-up surfaced after #316: //xff/glob:GlobToRegex duplicates mbo::file::Glob2Re2Expression / Glob2Re2 (mbo/file/glob.h, already in the pinned mbo), which #316 did not check before factoring the gitignore translator into a shared lib. Capture the known `**`/range semantic divergences and the decision to make (keep both / migrate onto mbo + delete //xff/glob / fix mbo upstream / keep ours), gated on ignore_test + the GLOB regex_test.
helly25
added a commit
that referenced
this pull request
Jul 11, 2026
…333) GlobToRegex's [...] scanner stopped at the first ']', mistranslating POSIX bracket syntax that RE2 supports natively: [[:ascii:]] -> [[:ascii:]\] (an ascii char, then a literal ]) []] -> []\] (RE2 error: empty class) [!]] -> similarly broken Ranges like [a-z] were already fine (passthrough). Fix: extract the class handling into AppendCharClass, which (1) treats a leading ']' as a literal member (escaped \] for RE2) and (2) recognizes [:class:] / [.collating.] / [=equiv=] sub-expressions and passes them through verbatim incl. their inner ']'. Standard POSIX-glob correctness - no new grammar, no change to '**' / gitignore semantics. Improves both --regextype=GLOB and the gitignore engine (both use GlobToRegex). The extraction also drops GlobToRegex's cognitive complexity 68 -> 31. Also documents in glob.h (resolving #122) that this is the gitignore-'**' dialect, kept separate from mbo::file::Glob2Re2 (a full FS-globbing lib with different '**' that xff does not use - it walks its own VFS engine), and that brace expansion is deliberately NOT here (a future opt-in shell-glob grammar; '{'/'}' stay literal in GLOB). Verified: glob_test (new POSIX-class + literal-] cases), regex/ignore/gitignore tests green; e2e --regextype=GLOB -name '[[:alpha:]]' and '[]]' match correctly.
helly25
added a commit
that referenced
this pull request
Jul 11, 2026
…#334) SHGLOB is a new match grammar: everything GLOB does, plus shell brace alternation, so `*.{cc,h}` matches either extension. As a matcher (not a shell) a brace group becomes an RE2 alternation - `*.{cc,h}` -> `[^/]*\.(?:cc|h)` - rather than expanding to several words. Why a separate grammar and not a GLOB feature: GLOB and the gitignore engine must keep matching literal `{`/`}` (a .gitignore line `foo{1}` matches literally), so brace expansion has to be opt-in. Named SHGLOB ("shell glob"), deliberately NOT EXTGLOB - in bash that names the pattern-lists ?(..)/@(..)/!(..), which we do not ship (the !(..) negation has no clean RE2 form). Semantics match bash: each alternative is itself SHGLOB-translated (nesting, `*`/`?`/`[...]` inside; alts may contain `/` and `**`); a comma-less `{x}` or an unbalanced `{` stays a literal brace; empty alternatives are allowed; `\{`/`\}`/`\,` escape to literals, as do `{`/`}`/`,` inside a `[...]` class. Deferred: numeric/char sequences {1..9}/{a..z} and extglob pattern-lists. Implementation is DRY: xff::glob gains an internal flavor-aware TranslateInto shared by GlobToRegex (braces off) and the new ShglobToRegex (braces on, recursive {...} parse); the `**`/`[...]` logic (incl. the #128 class scanner) is not duplicated. Wired through Grammar::kShglob (regex Compile), parser GrammarFromGlobals, run.cc ValidateRegextype (SHGLOB is a core engine, always linked), and the globals/doc-renderer SOT. Self-doc: --regextype gains a full details body documenting every grammar. It also gives PCRE2 the note it was missing - PCRE2 is the one build-time extra, present only in a full build, and `xff --help=extras` reports whether THIS binary has it (RE2/EXACT/FNMATCH/GLOB/ SHGLOB are always built in). Flows into --help=--regextype / --help=full / --man / --markdown. Also resolves #122: glob.h documents why xff keeps its own gitignore-`**` translator rather than migrating onto mbo::file::Glob2Re2 (different `**`; xff walks its own VFS engine). Tests: glob_test (ShglobToRegex + GLOB-braces-literal cases), regex_test (kShglob compile + match + path semantics), grep_test.sh (--regextype=SHGLOB end-to-end + GLOB literal-brace). Full suite green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopts helly25/mbo's pre-commit setup - the first of the two "CI elements" requested (trunk follows).
What
.pre-commit-config.yaml- generic hygiene (large-files, merge-conflict, check-yaml, eof-fixer, trailing-whitespace),actionlint,shfmt,shellcheck,clang-format, and local repo-policypygrephooks:no-do-not-merge,no-em-dashes,no-todos-without-context. Drops mbo repo-specific hooks (mope/bazelmod/compare-versions) that do not apply to xff; xff'sdonegate is already inline in main.yml.main.yml- newpre-commitjob (mirrors mbo: checkout + setup-python 3.13 +pre-commit/action@v3.0.1), wired into thedonegate. Removed the standalone no-em-dash grep (theno-em-dasheshook owns it now).explain_test.sh- shfmt normalized one inline-comment spacing.clang-format decision (re: "what runs clang-format?")
clang-format is pinned to v20.1.8 in pre-commit to match the hermetic toolchain clang-format (
tools/clang_format.sh) byte-for-byte (verified: zero reformatting). The hermeticclang-formatstep in thetestjob stays as the build-matched authority; if the LLVM toolchain ever bumps, the two versions disagree and CI fails loudly (a drift detector) rather than silently reformatting.Validation
pre-commit run -a: all 12 hooks green.bazel test //xff/cli/...:explain_test+style_testpass (shfmt change is comment-spacing only).Next: trunk (.trunk/) for clang-tidy + markdownlint/prettier/yamllint/buildifier, then apply clang-tidy via trunk.