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.

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

fix(parse): correct quotes matching for TermExec commands #52

Merged
merged 1 commit into from
Jun 20, 2021

Conversation

NTBBloodbath
Copy link
Contributor

Hey, after a while without using my TermExec commands I decided to trigger them and found that they weren't working properly since the command was null instead of the passed value (e.g. git status) when double quotes are used for enclosing the command. This PR aims to fix this behavior by checking two things before matching the command with the quoting.

  1. Checks if the current OS is Windows or not by using jit.os.
  2. Checks if the +shellslash feature exists and if it is turned on.

But, why check those things? Well, they're needed for some reasons:

  1. The Vimscript shellescape function works in a very peculiar way regarding the quoting.

From the shellescape function docs:

On Windows when 'shellslash' is not set, encloses {string} in double-quotes and doubles all double-quotes within {string}.
Otherwise encloses {string} in single-quotes and replaces all "'" with "'\''".

  1. The shellslash feature is only for Windows, that means we also need to check its value in addition to point 1 if the current OS is Windows because of how shellescape manages the quoting.

So basically, what was happening is that if you have used double quotes for enclosing your command the value:match(quotes) statement will return nil because the shellescape function enclosed the command in single quotes.

I only have tested in Linux because I don't have access to a Windows machine, but should work as expected too. Let me know if something needs to be changed.

@NTBBloodbath NTBBloodbath changed the title fix(parse): correct quotes matching for command fix(parse): correct quotes matching for TermExec commands Jun 19, 2021
@NTBBloodbath NTBBloodbath deleted the branch akinsho:master June 19, 2021 04:59
@NTBBloodbath NTBBloodbath deleted the master branch June 19, 2021 04:59
@NTBBloodbath NTBBloodbath restored the master branch June 19, 2021 04:59
@NTBBloodbath NTBBloodbath reopened this Jun 19, 2021
@akinsho
Copy link
Owner

akinsho commented Jun 20, 2021

@NTBBloodbath thanks for raising this PR, been a bit busy but will take a look later on today. Just to clarify, is this issue you are seeing just on Windows or also on Linux?

@akinsho
Copy link
Owner

akinsho commented Jun 20, 2021

@NTBBloodbath just tried it out and noticed what you meant, thanks for picking this up and fixing it. I realised all my tests and usage favoured single quotes, so I didn't pick this up

@akinsho akinsho merged commit 28c40c9 into akinsho:master Jun 20, 2021
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.

None yet

2 participants