make dependencies also work on windows #5

Merged
merged 1 commit into from Jun 27, 2013

2 participants

@sziep

with this change the dependencies can be specified in an OS agnostic way. So both "some/path" and "some\path" are working now.

@tJener
Owner

Hi! =D

Thanks for the pull request, it looks great. If you help me out with a couple small issues, I can merge this in posthaste.

  1. I'd like to keep the whitespace consistent, and will mark the specific lines where there are discrepancies.
  2. Could you squash/fixup your second commit into your first commit? I'd prefer to keep the history clean when possible.
@tJener tJener commented on an outdated diff Jun 27, 2013
tasks/lib/file-graph.js
@@ -143,7 +143,7 @@ exports.init = function( grunt ) {
}
if ( fileList ) {
- var files = _.words( split[1], ',' ).map( trimElems );
+ var files = _.words( split[1], ',' ).map( trimElems ).map(normalizePath);
@tJener
Owner
tJener added a line comment Jun 27, 2013

whitespace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tJener tJener commented on an outdated diff Jun 27, 2013
tasks/lib/file-graph.js
@@ -163,5 +163,9 @@ exports.init = function( grunt ) {
return _( str ).trim();
};
+ var normalizePath = function( p ) {
+ return p.replace(/[\/\\]+/g, path.sep);
@tJener
Owner
tJener added a line comment Jun 27, 2013

whitespace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tJener tJener commented on an outdated diff Jun 27, 2013
test/dep-concat_test.js
@@ -56,7 +56,7 @@ exports['dep-concat'] = {
basePath: 'test/fixtures/'
}, function( orderedFiles ) {
var indices = {};
- _.each([ 'main', 2, 5, 7, 8, 9, 10, 11 ], function( file ) {
+ _.each([ 'main', 2, 5, 7, 8, 9, 10, 11, 'somedir/12'], function( file ) {
@tJener
Owner
tJener added a line comment Jun 27, 2013

whitespace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tJener tJener commented on an outdated diff Jun 27, 2013
test/fixtures/9.js
@@ -1 +1,2 @@
-// Some file
+// file path windows style
+//load: somedir\\12.js
@tJener
Owner
tJener added a line comment Jun 27, 2013

Would this work with just one \?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sziep

Like so? :)

@tJener tJener merged commit 171e034 into tJener:master Jun 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment