Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Write curl body to a temporary JSON file #673

Merged
merged 3 commits into from
Oct 7, 2024
Merged

Write curl body to a temporary JSON file #673

merged 3 commits into from
Oct 7, 2024

Conversation

Huite
Copy link
Contributor

@Huite Huite commented Oct 1, 2024 •
edited
Loading

Fixes #672

Should have the same effect as #622, but cross-platform since this uses a tempname() provided by neovim instead.
(Somewhat surprising to me: the plenary curl wrapper checks whether the content of body is a file, and if so, it prepends @ telling curl to read the file contents. See: https://github.com/nvim-lua/plenary.nvim/blob/2d9b06177a975543726ce5c73fca176cedbffe9d/lua/plenary/curl.lua#L205)

I don't think you'd want to put the logic at the provider, since you'll have to repeat it for every provider. This will work for all providers since it's just calling curl in a slightly different way.

@trevorprater
Copy link

Also useful for debugging.

@yetone
Copy link
Owner

yetone commented Oct 5, 2024

LGTM, but there needs to be a cleanup logic for the temp file.

@Huite
Copy link
Contributor Author

Huite commented Oct 5, 2024 •
edited
Loading

I've added pcall(os.remove, temp_file) to delete the temporary file immediately after use.

(vim.fn.delete seems to run into "Vimscript function must not be called in a lua loop callback" issues)

For debugging, it might be worthwhile to store the temp_file path and not remove it immediately, but that would be a separate issue IMO.

@yetone
Copy link
Owner

yetone commented Oct 6, 2024

I've added pcall(os.remove, temp_file) to delete the temporary file immediately after use.

(vim.fn.delete seems to run into "Vimscript function must not be called in a lua loop callback" issues)

For debugging, it might be worthwhile to store the temp_file path and not remove it immediately, but that would be a separate issue IMO.

You can call vim.fn.delete within a vim.schedule to avoid this issue.

@yetone yetone merged commit 2a72dfa into yetone:main Oct 7, 2024
5 checks passed
@olimorris olimorris mentioned this pull request Nov 14, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: plenary curl fails to spawn process on Windows due to maximum command line length
3 participants
@Huite @trevorprater @yetone