Skip to content

feat: replace compile_commands_dirs with compile_commands_paths - #343

Merged
16bit-ykiko merged 1 commit into
clice-io:mainfrom
Myriad-Dreamin:commands-paths
Jan 10, 2026
Merged

16bit-ykiko merged 1 commit into
clice-io:mainfrom
Myriad-Dreamin:commands-paths

Conversation

@Myriad-Dreamin

@Myriad-Dreamin Myriad-Dreamin commented Jan 10, 2026 •

Copy link
Copy Markdown
Contributor

This PR adds compile_commands_paths and removes compile_commands_dirs. For each path in compile_commands_paths, it:

  • checks if the file entry at the path is a directory, and loads if there is a dir/compile_commands.json.
  • otherwise, reads the file content as a json compilation database at the path.

This PR removes compile_commands_dirs without considering compability with clangd, according to the offline discussion.

Summary by CodeRabbit

Release Notes

  • New Features
    • Compile commands configuration now supports both file and directory paths for more flexible compile_commands.json lookup, instead of directories only.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 10, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

A configuration parameter is renamed from compile_commands_dirs to compile_commands_paths across documentation, headers, and implementation to support both directory and file paths for compile_commands.json lookup.

Changes

Cohort / File(s) Summary
Configuration & Type Definitions
docs/clice.toml, include/Server/Config.h
Renamed config key and struct member from compile_commands_dirs to compile_commands_paths with updated description to reflect support for both files and directories.
Path Resolution Logic
src/Server/Lifecycle.cpp
Updated compile_commands loading to check if each path is a directory (load from within it) or a file (load directly). Added llvm::fs header for filesystem operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hops through paths both wide and small,
No more just folders—files answer the call! 🐰
From dirs to paths, the config takes flight,
Compile commands found left, right, and bright! 📋✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing compile_commands_dirs with compile_commands_paths across the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f30f68f and 11caa13.

📒 Files selected for processing (3)
  • docs/clice.toml
  • include/Server/Config.h
  • src/Server/Lifecycle.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: xmake / build (ubuntu-24.04, releasedbg)
  • GitHub Check: xmake / build (ubuntu-24.04, debug)
  • GitHub Check: xmake / build (macos-15, debug)
  • GitHub Check: xmake / build (windows-2025, releasedbg)
  • GitHub Check: xmake / build (macos-15, releasedbg)
  • GitHub Check: cmake / build (macos-15, Debug)
  • GitHub Check: clice / publish-clice (ubuntu-24.04, clice.tar.gz, clice-x86_64-linux-gnu.tar.gz, clice-symbol.tar.gz, cl...
  • GitHub Check: cmake / build (ubuntu-24.04, Debug)
  • GitHub Check: clice / publish-clice (windows-2025, clice.zip, clice-x64-windows-msvc.zip, clice-symbol.zip, clice-x64-w...
  • GitHub Check: clice / publish-clice (macos-15, clice.tar.gz, clice-arm64-macos-darwin.tar.gz, clice-symbol.tar.gz, clic...
  • GitHub Check: cmake / build (windows-2025, RelWithDebInfo)
🔇 Additional comments (4)
include/Server/Config.h (1)

24-24: LGTM! Field rename aligns with the new path-based configuration.

The rename from compile_commands_dirs to compile_commands_paths accurately reflects the expanded functionality that now supports both files and directories.

src/Server/Lifecycle.cpp (2)

3-4: LGTM! Filesystem header added to support directory checks.

The llvm/Support/FileSystem.h include is necessary for the new fs::is_directory() call introduced in the loading logic.


41-47: The code already has appropriate error handling. The load_compile_database() function (src/Compiler/Command.cpp:573-588) handles invalid or non-existent paths through llvm::MemoryBuffer::getFile(), which returns an error result that is explicitly checked and logged. When file operations fail, the function logs the error and returns an empty vector, allowing the program to continue gracefully.

Likely an incorrect or invalid review comment.

docs/clice.toml (1)

24-25: LGTM! Documentation accurately reflects the configuration changes.

The comment and key name have been updated to reflect that the configuration now accepts both files and directories for compile commands lookup. This aligns with the implementation changes in the codebase.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@16bit-ykiko
16bit-ykiko merged commit d6733dd into clice-io:main Jan 10, 2026
19 checks passed
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.

2 participants