Skip to content
This repository has been archived by the owner on Apr 20, 2018. It is now read-only.

Ignore extra characters at filename end in search #302

Closed
wants to merge 1 commit into from

Conversation

tastycode
Copy link

When working with certain bower assets (esp. webfonts). CSS paths may include extra characters intended to promote wide cross-browser compatibility and may not relate to the actual file on the file system. e.g.

//https://github.com/driftyco/ionicons/blob/v1.4.1/css/ionicons.css#L7
@font-face { font-family: "Ionicons"; src: url("../fonts/ionicons.eot?v=1.4.1"); src: url("../fonts/ionicons.eot?v=1.4.1#iefix") format("embedded-opentype"), url("../fonts/ionicons.ttf?v=1.4.1") format("truetype"), url("../fonts/ionicons.woff?v=1.4.1") format("woff"), url("../fonts/ionicons.svg?v=1.4.1#Ionicons") format("svg"); font-weight: normal; font-style: normal; }

In this case, the file name ../fonts/ionicons.eot?v=1.4.1 contains the suffix ?v=1.4.1. The actual file the revved finder needs to find is ionicons.eot. When the revvedfinder runs path.extname and path.basename on this file name. The extname is assigned .1 and the basename is assigned ionicons.eot?v=1.4. The file is not found, and not replaced in usemin. Eventually causing builds to contain incorrectly referenced font paths.

> var path = require('path');
undefined
> var filename = '../../ionicons.css?v=1.4.1';
undefined
> var extname = path.extname(filename)
undefined
> extname
'.1'
> var basename = path.basename(filename, extname)
undefined
> basename
'ionicons.css?v=1.4'

This PR will preprocess the filename to remove these characters so that revvedfinder can find the file.

Relevant: yeoman/yeoman#824 (comment)

@XhmikosR XhmikosR added this to the 2.2.0 milestone Apr 22, 2014
@XhmikosR
Copy link
Contributor

Does this supersede #243?

@XhmikosR XhmikosR added bug and removed bug labels Apr 22, 2014
@XhmikosR
Copy link
Contributor

Also, we need this to be rebased against the latest HEAD.

@sindresorhus
Copy link
Member

@tastycode ping ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants