Skip to content
This repository has been archived by the owner on Jan 28, 2020. It is now read-only.

Correctly escape the ^ for Windows #43

Merged
merged 5 commits into from Dec 10, 2019

Conversation

RomainLanz
Copy link
Contributor

@RomainLanz RomainLanz commented Dec 7, 2019

Hey 馃憢

This PR ensure that the execCommand correctly escape the ^ for Windows.

On Windows, the npm.cmd or yarn.cmd file will be called which is a Batch Script. In a Batch Script, the character ^ is the escaping character, which means it wasn't passed to the real npm/yarn.

Examples:

> npm.cmd install --save @eslint@^6.0.0

This command was in fact installing eslint@6.0.0.

Credits to @targos for the help.

@RomainLanz RomainLanz changed the title Windows arguments Correctly escape the ^ for Windows Dec 7, 2019
@codecov-io
Copy link

codecov-io commented Dec 7, 2019

Codecov Report

Merging #43 into master will not change coverage.
The diff coverage is 100%.

Impacted Files Coverage 螖
src/util/execCommand.js 100% <100%> (酶) 猬嗭笍
src/util/isWindows.js 100% <100%> (酶)
src/util/escapeArguments.js 100% <100%> (酶)

Copy link
Owner

@sapegin sapegin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request!

src/util/execCommand.js Outdated Show resolved Hide resolved
src/__tests__/npm.spec.js Show resolved Hide resolved
@RomainLanz
Copy link
Contributor Author

Hey @sapegin! 馃憢

It should be good.

I have also extracted the isWindows to check since we may need it somewhere else and it will be easier to mock.

src/util/execCommand.js Outdated Show resolved Hide resolved
src/util/escapeArguments.js Outdated Show resolved Hide resolved
src/util/execCommand.js Outdated Show resolved Hide resolved
@RomainLanz
Copy link
Contributor Author

Should be good by now!

Copy link
Owner

@sapegin sapegin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last minor tweak and it's ready to be merged ;-)

src/util/escapeCircumflexOnWindows.js Outdated Show resolved Hide resolved
Copy link
Owner

@sapegin sapegin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sapegin sapegin merged commit 69b6bd8 into sapegin:master Dec 10, 2019
@RomainLanz RomainLanz deleted the windows-arguments branch December 10, 2019 08:27
@sapegin
Copy link
Owner

sapegin commented Dec 10, 2019

馃帀 This PR is included in version 4.0.1 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants