Skip to content

Conversation

@michaldziurowski
Copy link
Contributor

Description

Previously, the return values of gsub were passed to p:write. This is incorrect because gsub returns two values: the changed data and the number of substitutions made. Since the method signature for write is write(txt, flag, mode), the number of substitutions returned by gsub was incorrectly passed as the flag attribute. This could lead to incorrect behavior.

Passing "w" explicitly means that the file will be opened for writing, and if it has any content, it will be truncated.

I also modified the test so that it would fail with the previous implementation. The reason for the failure is that when invoking write with the changed text and a flag of 1 (1 is the number of substitutions returned by gsub, and by accident it also corresponds to the O_WRONLY flag), the file is not truncated. Since the substitution text is shorter than the original file content, some leftovers remain in the file.

P.S. I've read the contributing guide but decided not to start a discussion since it seems like an obvious oversight. I hope you don't mind. :)

Checklist

  • I've read the contributing guidelines and have adhered to them in this PR
  • I've updated the README and/or relevant docs pages
  • I've run make docs to update the vimdoc pages

@olimorris
Copy link
Owner

Great catch and thanks for such a comprehensive PR. You're quite right about gsub and normally the LSP gets me out of trouble as I always forget this 😆.

@michaldziurowski
Copy link
Contributor Author

Great catch

As often is such situations it was pure luck :)
Thanks for your hard work and this great plugin.

@olimorris olimorris merged commit d5bc944 into olimorris:main Feb 20, 2025
4 checks passed
cleong14 pushed a commit to cleong14/codecompanion.nvim that referenced this pull request May 21, 2025
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