sebdeckers / grunt-rev

:punch: Asset revving for Grunt.js
MIT License
240 stars 54 forks source link

Postfix #2

Open sindresorhus opened 11 years ago

sindresorhus commented 11 years ago

@sleeper can you think of any reason why the format is 9e107d9d.default.txt instead of the more readable default.9e107d9d.txt? If not, we might consider switching.

sleeper commented 11 years ago

No, not really ... It think it has been done that way as it was the easiest to create while keeping the extension.

orand commented 11 years ago

This is simple enough to do with the following change to rev.js:

        var hash = md5(f, options.algorithm, 'hex'),
          postfix = hash.slice(0, options.length),
          ext = path.extname(f),
          renamed = [path.basename(f, ext), postfix, ext.slice(1)].join('.'),
          outPath = path.resolve(path.dirname(f), renamed);

This successfully renames the files but breaks grunt-usemin's ability to rewrite other files to point to the new name because its RevvedFinder.find method assumes the revved filename ends with the original filename. This code isn't as straight-forward to modify, especially if there's a desire to add a prefix/postfix config option to the rev task to maintain backward compatibility. In that case, RevvedFinder.find either needs to check for both naming conventions each time, or the usemin task needs to read the rev task's prefix/postfix config. The simplest solution would be to switch to the postfix-only convention in both usemin and rev.

sindresorhus commented 11 years ago

Backwards compat isn't a big concern since it's only the built files that are affected and shouldn't matter. Better to do it now than later.

orand commented 11 years ago

I agree, better sooner than later. I assume it would be best for this to go in @sleeper's grunt-usemin v2.0 branch if that's imminent. If not, I'd be glad to work on a PR to master.

A little care is needed during the transition since grunt-rev and grunt-usemin probably need to be upgraded in lockstep due to usemin's assumptions about rev's naming convention.

sindresorhus commented 11 years ago

Yes, 2.0 branch ;)