check File for absolute path on construction - #76
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe change adds an ChangesAPI documentation
SCM logging cleanup
Operating-system path tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@API.md`:
- Line 14: Fix the Table of Contents entry for Type Trait Helpers by either
adding the missing matching heading in API.md or updating the link to the
existing decltype documentation section, ensuring the fragment resolves
correctly.
In `@inc/mkn/kul/os.hpp`:
- Around line 211-224: Update the catch block in the File path-resolution
constructor after Dir::REAL() fails so it applies the same separator-based and
Dir::REAL_OR_NULL fallback resolution as the non-existing-path branch. Ensure
relative names ultimately use env::CWD() when no resolvable parent is found, and
never leave _d empty.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c48e0718-ddcd-4733-9459-0c4828eb968b
📒 Files selected for processing (3)
API.mdinc/mkn/kul/os.hppinc/mkn/kul/scm.hpp
| 1. [Core Definitions (`defs.hpp`)](#1-core-definitions-defshpp) | ||
| 2. [Exception Handling (`except.hpp`)](doc/mkn/kul/except.md) | ||
| 3. [Assertions (`assert.hpp`)](doc/mkn/kul/assert.md) | ||
| 4. [Type Trait Helpers (`decltype.hpp`)](#4-type-trait-helpers-decltypehpp) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the broken Table of Contents fragment.
Line 14 links to #4-type-trait-helpers-decltypehpp, but API.md has no matching heading. Add the missing section or point this entry to the actual decltype documentation page.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 14-14: Link fragments should be valid
(MD051, link-fragments)
🤖 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 `@API.md` at line 14, Fix the Table of Contents entry for Type Trait Helpers by
either adding the missing matching heading in API.md or updating the link to the
existing decltype documentation section, ensuring the fragment resolves
correctly.
Source: Linters/SAST tools
| if (is()) { | ||
| try { | ||
| this->_n = Dir::REAL(this->_n); | ||
| this->_d = Dir(Dir::PRNT(this->_n), m); | ||
| this->_n = this->_n.substr(_d.path().size() + 1); | ||
| } catch (mkn::kul::fs::Exception const& e) { | ||
| } | ||
| } else if (n.find(Dir::SEP()) != std::string::npos) { | ||
| this->_d = Dir(n.substr(0, n.rfind(Dir::SEP()))); | ||
| this->_n = this->_n.substr(n.rfind(Dir::SEP()) + 1); | ||
| } else if (auto const& _N = Dir::REAL_OR_NULL(this->_n)) { | ||
| this->_d = Dir(Dir::PRNT(*_N), m); | ||
| } else { | ||
| if (n.find(Dir::SEP()) != std::string::npos) { | ||
| this->_d = Dir(n.substr(0, n.rfind(Dir::SEP()))); | ||
| this->_n = this->_n.substr(n.rfind(Dir::SEP()) + 1); | ||
| } else { | ||
| if (auto const& _N = Dir::REAL_OR_NULL(this->_n)) this->_d = Dir(Dir::PRNT(*_N), m); | ||
| } | ||
| this->_d = Dir(env::CWD()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply path fallback after Dir::REAL() fails.
If the file is removed or becomes inaccessible between is() and Dir::REAL(), the catch block leaves _d empty. For a relative name, full() then uses Dir::JOIN("", _n), which adds a leading separator instead of using env::CWD().
After the catch, run the same separator and REAL_OR_NULL resolution used by the non-existing-path branch. Do not leave the File object with an empty directory.
🤖 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 `@inc/mkn/kul/os.hpp` around lines 211 - 224, Update the catch block in the
File path-resolution constructor after Dir::REAL() fails so it applies the same
separator-based and Dir::REAL_OR_NULL fallback resolution as the
non-existing-path branch. Ensure relative names ultimately use env::CWD() when
no resolvable parent is found, and never leave _d empty.
1 participant
Summary by CodeRabbit
Documentation
Bug Fixes
Tests