Skip to content

Enchancement: C and CPP support#102

Merged
runonthespot merged 9 commits into
BeaconBay:mainfrom
szavadsky:topic/cpp
May 24, 2026
Merged

Enchancement: C and CPP support#102
runonthespot merged 9 commits into
BeaconBay:mainfrom
szavadsky:topic/cpp

Conversation

@szavadsky
Copy link
Copy Markdown
Contributor

No description provided.

@runonthespot
Copy link
Copy Markdown
Contributor

Reviewed — really nice work. The 7-test suite covering C/C++ corner cases (template prefix merging, namespace breadcrumbs, contained-text suppression) is particularly strong. Approved.

The CI machinery was actually broken on main when you opened this — we fixed it in #110 (just merged). To get a meaningful green CI run on this PR, could you either:

  1. Merge main into your branch (the recommended path), or
  2. Tick 'Allow edits from maintainers' on the PR — we'll do the merge from this side.

Once CI is green I'll merge. Thanks again @szavadsky.

@szavadsky
Copy link
Copy Markdown
Contributor Author

@runonthespot thanks, done. Thanks for the outstanding software

@runonthespot
Copy link
Copy Markdown
Contributor

Heads-up — main moved a lot today (0.7.5/0.7.6/0.7.7 cut + a 20-pack of dependabot bumps merged via #125). This PR has a Cargo.lock conflict because the dependabot batch shifted many transitive versions.

Quickest fix: git merge main, then git checkout --theirs Cargo.lock && cargo check to regenerate the lock with your new tree-sitter-c/cpp entries. Followed by cargo fmt. I verified locally that the merged result builds clean.

If easier, say the word and I'll open a follow-up PR from my fork that supersedes this one (giving you full credit). Otherwise happy to wait.

@szavadsky
Copy link
Copy Markdown
Contributor Author

@runonthespot I feel a follow-up PR by you will be easier for both.

runonthespot added a commit that referenced this pull request May 24, 2026
Ships #133 (security: 11 CodeQL alerts + 4 npm transitives closed) and
#134 (clippy pedantic auto-fix cleanup across 27 files).

After this release: open security alerts = 0 across both Dependabot
and CodeQL surfaces.

Cutting partly so the in-flight community PRs (#102 C/C++ support, #104
markdown support) have a clean current base to rebase on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@runonthespot
Copy link
Copy Markdown
Contributor

Quick update @szavadsky — we're cutting 0.7.10 right now (ships shortly), and after that we'll hold off merging any further changes to main until both your PRs (#102 C/C++ + #104 markdown) are in. That way you only need to rebase once and don't have to keep chasing a moving target.

When you've got a moment, the rebase pattern is:

git fetch upstream  # or origin if your fork tracks BeaconBay/ck
git checkout your-branch
git merge upstream/main
# resolve any conflicts (likely just Cargo.lock — see my prior comment for the pattern)
cargo check --workspace && cargo fmt --all
git push

I already verified locally that both branches resolve cleanly against current main with just a Cargo.lock regeneration. Once you push, CI will run fresh and I'll merge as soon as it's green. Thanks for the patience and the excellent work.

@szavadsky
Copy link
Copy Markdown
Contributor Author

@runonthespot done

@szavadsky
Copy link
Copy Markdown
Contributor Author

@runonthespot done, as far as I can tell CI/CD issues are just setup?

@runonthespot
Copy link
Copy Markdown
Contributor

runonthespot commented May 24, 2026 via email

@runonthespot
Copy link
Copy Markdown
Contributor

Pushed two cleanup commits to your branch via maintainer-edits:

  • d76c6a2cargo fmt + cargo clippy --fix (12 collapsible_if warnings → if a && b)
  • 524b017 — dropped an accidentally-bundled SECURITY.md (oops, my workspace had it as untracked)

Verified locally: clippy clean at -D warnings, fmt clean, 47 ck-chunk tests pass.

Pre-merge review notes (3 findings, none blocking)

While CI re-runs, here are observations from a fresh deep review — all follow-up territory, none gate the merge:

  1. suppress_contained_text_chunks runs before fill_gaps in chunk_language. fill_gaps produces new Text chunks from unclaimed byte ranges inside class bodies (field declarations, access specifiers) and those aren't run back through the suppression pass. Net effect: things like int x; int y; in a class body can still emit as a standalone Text chunk. Probably want to either swap the order or pass gap chunks through suppression. Worth a follow-up PR.

  2. has_compound_statement filter in query_chunker.rs silently drops any function_definition without a compound_statement child. In tree-sitter-cpp, = default / = delete parse as declaration (not function_definition), so this filter does nothing for its apparent purpose. It also has non-zero false-positive risk in unusual header file constructions. Low urgency but might be worth a comment explaining the intent or removing if it's dead.

  3. merge_cpp_template_prefix_chunks byte-adjacency check requires template_chunk.span.byte_end == next_chunk.span.byte_start. A template prefix with intervening whitespace (multiline template<...> declarations, or even a blank line between the template clause and the definition) won't merge. Cosmetic but worth being aware of.

Test-coverage gaps to consider (not for this PR)

Header-only .h files, extern "C" { ... } blocks, anonymous namespaces, partial template specializations, K&R-style C, multi-line template parameter lists.

Once CI goes green I'll merge. Excellent work overall — the 9 tests are the strongest part and the namespace breadcrumb logic is well thought out.

Will follow up #104 once it's rebased on current main.

Mechanical pre-merge cleanup to get CI green:
- 6 collapsible_if patterns (cargo clippy --fix)
- rustfmt repositioning of the trailing { after the && chains

No logic changes.
@runonthespot runonthespot merged commit 5f48339 into BeaconBay:main May 24, 2026
9 checks passed
runonthespot added a commit to szavadsky/ck that referenced this pull request May 24, 2026
Conflicts resolved:
- CHANGELOG.md, README.md, docs-site language tables — kept both
  the C/C++ rows (from BeaconBay#102) and the Markdown row
- ck-chunk/src/lib.rs — added template-prefix merge (Cpp) and
  small-chunk merge (Markdown) side by side; expanded the
  decorator-like trivia kinds match arm to include Markdown
- Cargo.lock auto-resolved by cargo via the workspace deps

Plus mechanical clippy --fix for 2 new warnings introduced by the
merge interaction (needless_borrow + manual_range_contains).
runonthespot added a commit that referenced this pull request May 24, 2026
Headline: C, C++, and Markdown language support land via #102 and #104
(both by @szavadsky). Plus the docs-deploy fix (#135) that resolves
the vitepress build break caused by my unbounded vite override in 0.7.10.

Also the live test of npm trusted publishing now that the repo's
trusted-publisher config is verified saved on the npm side.

See CHANGELOG.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants