feat(system): use last 2 folder layers for project name#8
Conversation
Previously, get_project_name() only returned the last folder layer (e.g., "coding-extension-vim"). This caused context loss when multiple projects have similar subfolder names like "frontend". Now returns the last 2 layers (e.g., "malamtime/coding-extension-vim") for better project identification in heartbeat data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the system's project naming logic by extending the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the get_project_name function to return the last two directory layers of a path, which is a great improvement for project identification. The implementation logic is sound and handles various path structures correctly. The tests have been updated accordingly, but I've identified a test case that has become brittle due to its dependency on the test execution environment. I've provided a suggestion to improve the test's robustness. Overall, this is a solid feature enhancement.
| -- Should contain a slash for 2 layers | ||
| assert.is_true(name:find('/') ~= nil) |
There was a problem hiding this comment.
This assertion makes the test brittle because it depends on the environment where the tests are run. For example, if the project is located in a root directory like /my-project, get_project_name will correctly return my-project, but this assertion will fail because the name does not contain a /.
Test cases should be deterministic. I recommend removing this check and refactoring the parent test it('should handle current project', ...) to use hardcoded paths that cover specific scenarios. For example, you could add new tests for paths like /foo/bar and the root path /.
Code ReviewSummaryThis PR improves project identification by using the last 2 folder layers instead of just 1. The implementation is clean and the tests are updated appropriately. Strengths
Potential Issues1. Windows compatibility concern (lua/shelltime/utils/system.lua:89)The current implementation checks parent != '/' which works for Unix paths, but on Windows, the root could be C:\ or other drive letters. This may cause incorrect behavior on Windows. 2. Trailing slash handling (lua/shelltime/utils/system.lua:87-88)If project_root has a trailing slash, the behavior might be unexpected. While fnamemodify typically handles this, it is worth testing. 3. Edge case: Root directory projects (tests/system_spec.lua:200)The test assertion assumes all project paths will contain a slash. This would fail if the current project is only one level deep. Consider making it conditional or removing it. SuggestionsPerformance: The implementation is fine - fnamemodify is efficient and this function is not called frequently. Code Quality: Consider extracting the root detection logic to improve testability and Windows compatibility. Security: No security concerns identified. Test Coverage: Good overall. Suggested additions:
Overall AssessmentRecommendation: Approve with minor changes The PR is well-implemented and solves a real problem. The main concerns are:
The core logic is sound and follows the project coding conventions well. Nice work! Review generated by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Summary
get_project_name()to return last 2 folder layers instead of just 1/code/malamtime/coding-extension-vimnow returnsmalamtime/coding-extension-viminstead of justcoding-extension-vimTest plan
./scripts/test.sh🤖 Generated with Claude Code