fix: not work in gitbash

[iamxiaojianzheng]

about this #36

[oysandvik94]

@iamxiaojianzheng Thank you for finding the error, I really appreciate it!
There are some tests failing because it's asserting the command that is being run, and since it's a table instead of a string they now fail.

Do you want me to do it for you?

[iamxiaojianzheng]

@iamxiaojianzheng Thank you for finding the error, I really appreciate it! There are some tests failing because it's asserting the command that is being run, and since it's a table instead of a string they now fail.

Do you want me to do it for you?

I hope you can check again whether this change is reasonable.

On your computer environment, try to see if this change is compatible.

[oysandvik94]

@iamxiaojianzheng

It worked on my machine, so the change is good :)
Only issue is that the unit tests are failing, but this is just an error in the test itself, so we can change the test.

You can see them here: https://github.com/oysandvik94/curl.nvim/actions/runs/10557596972/job/29245918019?pr=39

[iamxiaojianzheng]

@iamxiaojianzheng

It worked on my machine, so the change is good :) Only issue is that the unit tests are failing, but this is just an error in the test itself, so we can change the test.

You can see them here: https://github.com/oysandvik94/curl.nvim/actions/runs/10557596972/job/29245918019?pr=39

Sorry, I'm not familiar with unit testing. How should I handle this error?

[oysandvik94]

@iamxiaojianzheng Actually, I see one more issue:

If you try to run:

curl -X POST https://jsonplaceholder.typicode.com/posts
-H 'Content-Type: application/json'
-d
{
  "title": "now try this"
}

Then you get an error:
curl: option : blank argument where content is expected curl: try 'curl --help' for more information

[oysandvik94]

After thinking a little bit, I think the right way to do this is to built up the command table bit by bit, instead of creating a string that is then finally split.
I can have a look at this tonight.

[iamxiaojianzheng]

After thinking a little bit, I think the right way to do this is to built up the command table bit by bit, instead of creating a string that is then finally split. I can have a look at this tonight.

Yes, tested. I found that needed to parse curl's syntax and build a reasonable list of command arguments.

[iamxiaojianzheng]

@oysandvik94

I seem to have solved the problem of executing command.

You tried to execute the bad command, there should be a bug.

unexpected EOF while looking for matching `''

My guess is that the format_command_for_curl method is causing the problem, which makes the last command executed missing a single quote.

# bad
curl -X POST https://jsonplaceholder.typicode.com/posts 
-H
'Content-Type: application/json'
-d
{ "title": "now try this" }

# good
curl -X POST https://jsonplaceholder.typicode.com/posts
-H 'Content-Type: application/json'
-d
{
  "title": "now try this"
}

# good
curl -X POST https://jsonplaceholder.typicode.com/posts
-H 'Content-Type: application/json'
-d
{ "title": "now try this"
}

# good
curl -X POST https://jsonplaceholder.typicode.com/posts
-H 'Content-Type: application/json'
-d
'{"title": "now try this"}'

[oysandvik94]

I think maybe the "right" solution here is to rewrite the parse_curl_command function to parse all the components of the curl command in to elements in a table, but this requires a bigger rewrite.

I think your solution is fine in the meantime.

However, my worry here is that we need to support all shells. What would happen in this case if a user had fish? We should at least have a sensible default. See my comment in the PR.

The tests are also failing due to the curl command being changed. If you need to run tests locally, run the "tests" script. From the root of the project, run:
./tests/run

[oysandvik94]

Can we just return an empty table here, so that we do a best effort attempt to run the command if a shell is not found?

[iamxiaojianzheng]

I tried the {"curl xxxx"} format and it doesn't seem to work.

[iamxiaojianzheng]

I thought it would be nice to highlight some of the shell notes in the README before the refactoring is complete.

[iamxiaojianzheng]

Or for users with an unknown shell, can provide a configuration to be compatible with it.

[oysandvik94]

Could we maybe say that if there's no found shell, it reverts to the previous behaviour?
I really don't want to have the plugin break for people because their shell is not defined in our code, when we don't actually need do anything for it to work.

Also, I dont think this plugin works with powershell since it relies on JQ, which I dont think can be installed in Powershell. This might give the impression that powershell is supported.

[iamxiaojianzheng]

No problem, if getting the shell fails, just execute the string as before, not the table.

[iamxiaojianzheng]

I can't execute this script locally and I don't know why.

[oysandvik94]

Strange, might be OS related? I copied the busted file from folke's plugins. You'll see the test errors in the github action run of this PR.

[iamxiaojianzheng]

I think if you want to refactor, you can consider changing the design of the interactive UI.

The current use and information presentation, I think a little too simple.

I think you can look at rest.nvim and kulala.nvim to refine the interaction design of curl.nvim.

[oysandvik94]

@iamxiaojianzheng I'm happy with the default case here.

There is only one test failure left, in "api_spec.lua". Line 13 should be:

		local mocked_jobstart = function(command, callback)
			test_util.assert_equals(curl_command .. " -sSL", command[#command])

			buffers.set_output_buffer_content(0, { "test" })
		end

As for your point on design/ux: This is definitely a minimalistic plugin compared to those you mention. It would be nice to get something more fancy in the future, but for now I am focusing on functionality and stability. When I get more time, I might invest more time in UX.
Of course open to contributions in this area.

[iamxiaojianzheng]

@iamxiaojianzheng I'm happy with the default case here.

There is only one test failure left, in "api_spec.lua". Line 13 should be:

		local mocked_jobstart = function(command, callback)
			test_util.assert_equals(curl_command .. " -sSL", command[#command])

			buffers.set_output_buffer_content(0, { "test" })
		end

As for your point on design/ux: This is definitely a minimalistic plugin compared to those you mention. It would be nice to get something more fancy in the future, but for now I am focusing on functionality and stability. When I get more time, I might invest more time in UX. Of course open to contributions in this area.

I have modified

[oysandvik94]

All tests are passing and it looks good. Thank you so much for your contribution!
I will merge this, and this closes #39