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

Use "replaceall" for relative -> absolute path replacement #35

Merged
merged 1 commit into from
Mar 7, 2016

Conversation

Turbo87
Copy link
Contributor

@Turbo87 Turbo87 commented Mar 6, 2016

This PR resolves #29 again...

The issue was fixed in #33 but only for cases where there is only a single file with a lint error because the replace() method in JS only replaces the first instance by default... 🤦

replace() only replaces the first instance by default which is still causing the original issue if there is more than one file with a lint error
@IanVS
Copy link
Contributor

IanVS commented Mar 7, 2016

Hm, I'm not sure it's worth pulling in a dependency for this. Couldn't we just use a global regexp?

var cwd = process.cwd() + '/';
var re = new RegExp(cwd, 'g');
formatter(report.results).replace(re, '')

That's basically all replaceall is doing anyway.

@Turbo87
Copy link
Contributor Author

Turbo87 commented Mar 7, 2016

@IanVS the problem is that process.cwd() will likely contain "special" characters like slashes and backslashes which are interpreted as regex patterns if they are not escaped properly.

we could obviously just copy-paste the replaceall code into this repo, but then again what's the point if we can just declare the dependency.

upstream fix for mocha is pending since earlier today: mochajs/mocha#2145

@IanVS
Copy link
Contributor

IanVS commented Mar 7, 2016

Ah, that makes sense. I'll merge this for now and we can check if it's still necessary once your upstream is merged. Thanks!

IanVS added a commit that referenced this pull request Mar 7, 2016
Use "replaceall" for relative -> absolute path replacement
@IanVS IanVS merged commit b9c47f7 into BadgeLabs:master Mar 7, 2016
@Turbo87
Copy link
Contributor Author

Turbo87 commented Mar 7, 2016

Cool, thanks!

@Turbo87 Turbo87 deleted the fix-paths-bug branch March 7, 2016 13:34
@Turbo87
Copy link
Contributor Author

Turbo87 commented Mar 15, 2016

are you planning to release this as v2.0.2?

@IanVS
Copy link
Contributor

IanVS commented Mar 15, 2016

Thanks for the reminder. v2.0.2 is now released.

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.

Errors are reported twice
2 participants