Really put the hash before the extension

[XhmikosR]

Second attempt, but we really need tests.

With master:

{
  "dist/css/site.test.css": "dist/css/site-a55e80c2ce.test.css",
  "dist/css/site.test.min.css": "dist/css/site-a55e80c2ce.test.min.css",
  "dist/jquery.fancybox.min.css": "dist/jquery-1679dcd38f.fancybox.min.css",
  "dist/jquery.fancybox.min.js": "dist/jquery-b762d7a222.fancybox.min.js"
}

This branch:

{
  "dist/css/site.test.css": "dist/css/site-a55e80c2ce.test.css",
  "dist/css/site.test.min.css": "dist/css/site.test-a55e80c2ce.min.css",
  "dist/jquery.fancybox.min.css": "dist/jquery.fancybox-1679dcd38f.min.css",
  "dist/jquery.fancybox.min.js": "dist/jquery.fancybox-b762d7a222.min.js",
}

As you can see the period issue isn't fixed in master either.

[sindresorhus]

dist/jquery.fancybox-1679dcd38f.min.css should be dist/jquery.fancybox.min-1679dcd38f.css

[sindresorhus]

Second attempt, but we really need tests.

Indeed, but I don't really have any time to spend on this plugin.

[sindresorhus]

// @phazei

[XhmikosR]

I spent some time trying to debug this and it seems this comes from somewhere else and not this plugin. Maybe modify-filename?

[sindresorhus]

Maybe modify-filename?

Nope https://github.com/sindresorhus/modify-filename/commit/1a86d0d5ce879743e1a06d03b55073a123985a4a

[XhmikosR]

OK then only revPath is left :)

filename: site.test
extension: .css
revPath(filename, file.revHash): site-a55e80c2ce.test
filename: site.test.min
extension: .css
revPath(filename, file.revHash): site.test-a55e80c2ce.min

[XhmikosR]

OK I give up again. It's not revPath either. It's something in the plugin. I know for sure filename and extension are correct in this branch in transformFilename(). After that it breaks somewhere.

[XhmikosR]

Wait!

Aren't we supposed to pass a path in revPath? Currently the filename is passed.

[XhmikosR]

@sindresorhus please check out the last patch. Basically I kept the old behavior for .map files only.

Can you add a test please?

I'm thinking though perhaps we should keep the hash before .min also.

[sindresorhus]

Why should .map files have special behavior?

[XhmikosR]

Don't ask me, you do that :P

[XhmikosR]

OK I think this is ready now. But please someone add tests because I have no idea about ava etc

With master:

{
  "dist/site.test.css": "dist/site-a55e80c2ce.test.css",
  "dist/site.test.min.css": "dist/site-a55e80c2ce.test.min.css"
}

With this patch:

{
  "dist/site.test.css": "dist/site.test-a55e80c2ce.css",
  "dist/site.test.min.css": "dist/site.test-a55e80c2ce.min.css"
}

[sindresorhus]

I think it's better to not have any special rules. That makes it more predictable.

[XhmikosR]

I agree with that, but 1) for map files they are already special if you check the code and 2) to be honest it just looks more natural to me to keep min after the hash.

[sindresorhus]

1. This plugin does make them, but they should still follow the same rules as other files.
2. Predictability > How it looks.

This looks good to be merged when you've removed the special-cases.

[XhmikosR]

I'm not sure I follow. The current code already treats map files specially. https://github.com/sindresorhus/gulp-rev/blob/master/test/rev.js#L29-L30

As for the min part, it's the same convention, which I didn't invent either. It's so that it's a lot easier to target them without all kinds of regexes.

[XhmikosR]

@sindresorhus: so, what's going to be with this? Note that the current patch is not breaking in the sense that it'll keep the same behavior as before for map and min files, which is expected IMO.

[sindresorhus]

As for the min part, it's the same convention, which I didn't invent either. It's so that it's a lot easier to target them without all kinds of regexes.

Where do you stop with the special cases though? Soon someone would want a special case for .tar.gz and any other commonly used file type with multiple extensions, and then we're just back to scratch.

[XhmikosR]

That's true, so I guess these are the only cases.

[sindresorhus]

Closing for lack of activity.