Skip to content

Fix absolute paths#203

Merged
XhmikosR merged 1 commit into
gruntjs:masterfrom
smcgivern:fix_absolute_paths
Apr 23, 2015
Merged

Fix absolute paths#203
XhmikosR merged 1 commit into
gruntjs:masterfrom
smcgivern:fix_absolute_paths

Conversation

@smcgivern
Copy link
Copy Markdown

This should fix #191 and maybe #196 too.

The relativeTo option was removed at https://github.com/gruntjs/grunt-contrib-cssmin/pull/170/files#diff-f0ee9eb300066e1e51fcfbad9d7a78e1L45 and seemingly never added back.

@XhmikosR
Copy link
Copy Markdown
Member

XhmikosR commented Apr 6, 2015

/CC @sindresorhus

Comment thread Gruntfile.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should use path.join().

@smcgivern
Copy link
Copy Markdown
Author

Good points. Do you mind if I just amend the existing commit, or would you prefer a separate one?

@XhmikosR
Copy link
Copy Markdown
Member

XhmikosR commented Apr 6, 2015

@smcgivern: just amend.

@smcgivern smcgivern force-pushed the fix_absolute_paths branch from 81586b2 to 4deac5b Compare April 6, 2015 18:16
@smcgivern
Copy link
Copy Markdown
Author

OK, done 👍

@jd-carroll
Copy link
Copy Markdown

I know this is already merged, but..

Why are you setting:

options.relativeTo = path.dirname(availableFiles[0]);

When it should be relative to the destination:

options.relativeTo = options.target = file.dest;

@smcgivern
Copy link
Copy Markdown
Author

Honestly, because it was like that before:

options.relativeTo = path.dirname(file);
In the case here, where we're using absolute paths, it doesn't look like it matters. A test case that shows why this is the wrong behaviour is probably the best thing to provide.

This also isn't merged - @XhmikosR a little help please? 😃

XhmikosR added a commit that referenced this pull request Apr 23, 2015
@XhmikosR XhmikosR merged commit fe59f98 into gruntjs:master Apr 23, 2015
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.

grunt-contrib-cssmin creating min.css empty files

5 participants