-
-
Notifications
You must be signed in to change notification settings - Fork 252
refactor(create_file): use vim.uv
functions
#1642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- replace plenary.path with native vim functions for file and directory operations - update file existence and directory checks to use vim.fn - handle parent directory creation and file writing with vim.fn methods
- add empty string to expected output to match file with trailing newline
The related disscussion: #1641 |
Actually, I tried to fix the plenary problem, but honestly, the plenary's code is a bit hard to maintain |
This is the first I've heard of this and we've had this functionality in the plugin for over a year now. Can you cross-reference the Plenary issue that exists for this. Plenary is deeply embedded in the plugin so I'm surprised this is the one area you've been experiencing problems with. |
Reproduction is easy, just ask codecompanion to use @files to create a non-existent file (the parent folder does not exist either) like: |
I don't have a Windows machine so reproduction isn't easy at all. This needs to be a bug report filed with Plenary so someone can fix this upstream. The changes in this PR are minimal but I'm unwilling to change core functionality because a user says they're having a problem without providing any evidence or willing to fix it for the wider community. As I mentioned before, this is the first time I've had a windows issue raised with this functionality. |
The main reason is that there is a problem with the parsing parent function of plenary. I see that it uses regular matching |
Unfortunately, plenary doesn't seem to be developing actively at the moment, and it uses more ancient methods, such as using vim.loop In fact, throughout 2024, Plenary has few submissions, and many PRs have been unaudited and not merged. And this is plenary releated issue: nvim-lua/plenary.nvim#393 Despite claims to be repaired, similar issues remain |
I'll change this pr to use uv later |
- replace vim.fn with vim.uv for file and directory checks - add recursive directory creation using uv - improve error handling for file creation and writing - ensure file descriptors are properly closed
- remove trailing empty string from expected output - ensure test matches actual file content
vim.uv
functions
Now use vim.uv and it can ensure that the function is the same as it was originally |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I accept this is a Plenary issue. However, for this to be added into CodeCompanion I want this to exist as a utility file - something that we can reuse across the plugin. It's no use existing in one tool if this issue is going to arise again, elsewhere in the plugin, in the future.
The impact of this is we will need test coverage for that utility file as well.
So unfortunately there's a bit of work to get this PR in a state that I'd be happy to accept.
OK, then according to my understanding, what I need to do next is
|
- implement files.create_dir_recursive for robust directory creation - refactor create_file tool to use new utility and improve error handling - add comprehensive tests for create_dir_recursive, including edge cases and logging
- move require("codecompanion.utils.log") after files import - adjust require order in test_files.lua for readability
Maybe now the requirement is met |
- replace describe/it blocks with MiniTest.new_set structure - update test hooks to pre_case and post_case - migrate all create_dir_recursive tests to new MiniTest style - improve Windows root directory test with MiniTest.skip - update log.error mocks for compatibility with new test style - return test set at end of file
- remove nested pcall and use direct error checks - provide clearer error messages for directory, file open, write, and close failures - combine write and close errors for comprehensive reporting - streamline control flow for better readability
I've corrected the code according to the specification I carefully checked the specifications of other codes and the implementation is really great Thank you for your work |
I also tried to run unit tests on Windows, but found that there were basically no errors. The code specifications were written really well. The problem was that there were some problems with the backslash processing, but it did not affect the use, but it would only cause some unit tests to be unable to pass on Windows |
- add vim.fs.normalize to ensure consistent file paths - prevent potential issues with unnormalized paths
Description
vim.uv
functions for file and directory operationsPlenary creates files on Windows often fail to create parent successfully.
Using vim's built-in method is more convenient
Related Issue(s)
Checklist
CodeCompanion.has
in the init.lua file for my new featuremake all
to ensure docs are generated, tests pass and my formatting is applied