Repository navigation
feat(hover): show include paths and TagDecl members - #497
zaragoza-xu wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis PR extends hover rendering to show full record and enum definitions and adds hover information for ChangesHover Feature Enhancements
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant hover_info
participant include_hover
Editor->>hover_info: request hover at offset
hover_info->>include_hover: check include argument range
include_hover-->>hover_info: return Header hover with resolved path
hover_info-->>Editor: return hover result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
zaragoza-xu
changed the title
There was a problem hiding this comment.
Pull request overview
Adds richer hover support in the C++ feature layer by introducing header/include directive hovers and expanding type hover output to include enum constants and record fields, with accompanying tests/snapshots/docs updates.
Changes:
- Added include-directive hover support that returns header name, resolved path, and directive argument range.
- Updated hover definition rendering for records/enums to include record fields and enum enumerators (with computed values for implicit enumerators).
- Added/updated unit tests, hover snapshots, and documentation checklists for the new hover behaviors.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/feature/hover_tests.cpp | Adds a unit test covering hover behavior on #include header arguments (name/path/range + rendered hover content). |
| tests/snapshots/hover/snapshot/tag_decls.cpp.snap.yml | Updates expected hover output to include enum constants and record fields in rendered definitions/markdown. |
| tests/data/hover/tag_decls.cpp | Extends hover test input data with explicit-enum and struct-with-fields cases used by snapshots. |
| src/feature/hover.cpp | Implements include directive hover; changes record/enum definition printing to include members; wires include hover into hover_info. |
| docs/zh/features/hover.md | Marks the struct/enum-members-on-hover item as completed in the Chinese docs. |
| docs/en/features/hover.md | Marks the struct/enum-members-on-hover item as completed in the English docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| os << " {"; | ||
| for(const clang::FieldDecl* field: decl.fields()) { | ||
| os << '\n'; | ||
| field->print(os, policy); | ||
| os << ';'; |
| auto try_directive = [&](clang::SourceLocation loc, | ||
| clang::FileID target) -> std::optional<HoverInfo> { | ||
| if(!target.isValid()) | ||
| return std::nullopt; | ||
| auto [fid, directive_offset] = unit.decompose_location(loc); | ||
| if(fid != interested || directive_offset >= content.size()) | ||
| return std::nullopt; | ||
| auto range = find_directive_argument(content, directive_offset, lang_opts); | ||
| if(!range || !range->contains(offset)) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/zh/features/hover.md (1)
269-269: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMark the
#includedirective hover checkbox as implemented (Chinese docs).Same as the English docs, line 269 still shows
- [ ]#include指令悬停显示解析后的头文件路径as unimplemented, but this PR implements this feature. Update to[x]for consistency.📝 Proposed fix
-- [ ] `#include` 指令悬停显示解析后的头文件路径 +- [x] `#include` 指令悬停显示解析后的头文件路径🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/zh/features/hover.md` at line 269, The Chinese hover docs still mark the `#include` directive hover feature as unimplemented, but it is now implemented. Update the checkbox item in the hover documentation so the `#include` directive entry is marked as completed, keeping it consistent with the English docs and the implemented behavior.docs/en/features/hover.md (1)
269-269: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMark the
#includedirective hover checkbox as implemented.Line 269 still shows
- [ ]#includedirective hover showing resolved header pathas unimplemented, but this PR adds theinclude_hoverfunction and a dedicatedinclude_headerunit test that verifies exactly this behavior. The checkbox should be updated to[x]to reflect the implementation.📝 Proposed fix
-- [ ] `#include` directive hover showing resolved header path +- [x] `#include` directive hover showing resolved header path🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/en/features/hover.md` at line 269, The hover documentation still marks the `#include` directive hover as unimplemented, but this feature is already covered by `include_hover` and the `include_header` unit test. Update the checkbox in the hover docs from unchecked to checked so the status matches the implemented resolved-header-path behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/feature/hover.cpp`:
- Line 1148: `include_hover` can assign `SymbolKind::Header`, but
`symbol_kind_string` currently falls through to the empty default for that kind,
so `HoverInfo::present()` omits the label. Update the `symbol_kind_string`
switch in `HoverInfo::present()` handling to include a `SymbolKind::Header` case
that returns the header label string, alongside the existing symbol kinds.
---
Outside diff comments:
In `@docs/en/features/hover.md`:
- Line 269: The hover documentation still marks the `#include` directive hover
as unimplemented, but this feature is already covered by `include_hover` and the
`include_header` unit test. Update the checkbox in the hover docs from unchecked
to checked so the status matches the implemented resolved-header-path behavior.
In `@docs/zh/features/hover.md`:
- Line 269: The Chinese hover docs still mark the `#include` directive hover
feature as unimplemented, but it is now implemented. Update the checkbox item in
the hover documentation so the `#include` directive entry is marked as
completed, keeping it consistent with the English docs and the implemented
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6938605-ee44-41b7-9615-02ea191f077f
📒 Files selected for processing (6)
docs/en/features/hover.mddocs/zh/features/hover.mdsrc/feature/hover.cpptests/data/hover/tag_decls.cpptests/snapshots/hover/snapshot/tag_decls.cpp.snap.ymltests/unit/feature/hover_tests.cpp
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 182328140a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
zaragoza-xu
changed the title
0352b94 to
2375118
Compare
zaragoza-xu
changed the title
zaragoza-xu
changed the title
9f548cb to
d3f5b53
Compare
This pull request significantly improves the hover feature in the codebase by enhancing the information shown when hovering over struct, enum, and
#includedirectives. The main changes include implementing detailed type-level hover for structs/enums, adding hover support for#includedirectives to show resolved header paths, and updating documentation and tests to reflect these new capabilities.Hover Feature Enhancements:
src/feature/hover.cpp,tests/data/hover/tag_decls.cpp,tests/snapshots/hover/snapshot/tag_decls.cpp.snap.yml) [1] [2] [3] [4] [5] [6] [7]#includedirectives, showing the resolved header path when hovering over the header name. (src/feature/hover.cpp,tests/unit/feature/hover_tests.cpp) [1] [2] [3] [4]Documentation Updates:
#includedirective hover as implemented. (docs/en/features/hover.md,docs/zh/features/hover.md) [1] [2] [3] [4]Testing Improvements:
#includedirective hover. (tests/data/hover/tag_decls.cpp,tests/snapshots/hover/snapshot/tag_decls.cpp.snap.yml,tests/unit/feature/hover_tests.cpp) [1] [2] [3]These changes collectively provide much richer and more informative hover experiences for users, especially when working with complex types and include directives.