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

Allow backslash #173

Merged
merged 2 commits into from
Mar 9, 2018
Merged

Allow backslash #173

merged 2 commits into from
Mar 9, 2018

Conversation

greggman
Copy link
Contributor

@greggman greggman commented Mar 9, 2018

see #167

Not really sure what you want the solution to be. Apparently you want cross-env to use backlash as an esapce character even though Windows itself does not do this. So, this patch lets esaped backslashes through so if you want to pass \\someshare\somefolder you'd have to pass \\\\someshare\\somefolder as the argument.

also i tried running npm run add-contributor but appearently it doesn't work at the moment

What: allow backslashes through

Why: beacuse otherwise you can't pass backslash through and can not specify UNC paths

How: adjust the regular expression related to backslashes

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table
C:\Users\gregg\src\cross-env>npm run add-contributor

> [email protected] add-contributor C:\Users\gregg\src\cross-env
> kcd-scripts contributors add

module.js:557
    throw err;
    ^

Error: Cannot find module 'all-contributors-cli/cli'
    at Function.Module._resolveFilename (module.js:555:15)
    at Function.resolve (internal/module.js:18:19)
    at Object.<anonymous> (C:\Users\gregg\src\cross-env\node_modules\kcd-scripts\dist\scripts\contributors.js:7:33)
    at Module._compile (module.js:660:30)
    at Object.Module._extensions..js (module.js:671:10)
    at Module.load (module.js:573:32)
    at tryModuleLoad (module.js:513:12)
    at Function.Module._load (module.js:505:3)
    at Function.Module.runMain (module.js:701:10)
    at startup (bootstrap_node.js:190:16)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] add-contributor: `kcd-scripts contributors add`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] add-contributor script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\gregg\AppData\Roaming\npm-cache\_logs\2018-03-09T03_13_59_441Z-debug.log

@codecov
Copy link

codecov bot commented Mar 9, 2018

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #173   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines          63     64    +1     
  Branches       14     15    +1     
=====================================
+ Hits           63     64    +1
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5204dd6...e40b572. Read the comment docs.

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

This looks fine to me. But to be perfectly honest I feel like I've sorta lost control of this project, so I'd really appreciate it if someone else reviews this as well as a sanity check because this thing has become way more complicated than it ever should have I feel...

@kentcdodds
Copy link
Owner

Come to think of it, did you try to use cross-env-shell?

@greggman
Copy link
Contributor Author

greggman commented Mar 9, 2018

I didn't try cross-env-shell but trying it now it has the same issue

For my particular issue I've just stopped using cross-env when I need to pass a UNC

What I type normally

npm run somescript some\path

But when I need to run with a UNC path then I have to do whatever somescript was doing manually

cmd /S /C "set SOME_ENV=somevalue & node path/to/script.js \\someunc\path"

@kentcdodds
Copy link
Owner

Well, it seems reasonable to me.

If this breaks anyone, I'm really sorry. Please offer to help maintain the project :)

@kentcdodds kentcdodds merged commit 450dae9 into kentcdodds:master Mar 9, 2018
This was referenced Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants