Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature rewrite-urls #3041

Closed
wants to merge 11 commits into from
22 changes: 21 additions & 1 deletion Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,18 @@ module.exports = function (grunt) {

var sauceJobs = {};

var browserTests = [ 'filemanager-plugin',

var browserTests = [
'filemanager-plugin',
'visitor-plugin',
'global-vars',
'modify-vars',
'production',
'rootpath-relative',
'rootpath-rewrite-urls',
'rootpath',
'relative-urls',
'rewrite-urls',
'browser',
'no-js-errors',
'legacy'
Expand Down Expand Up @@ -358,6 +362,14 @@ module.exports = function (grunt) {
outfile: 'tmp/browser/test-runner-relative-urls.html'
}
},
rewriteUrls: {
src: ['test/browser/less/rewrite-urls/*.less'],
options: {
helpers: 'test/browser/runner-rewrite-urls-options.js',
specs: 'test/browser/runner-rewrite-urls-spec.js',
outfile: 'tmp/browser/test-runner-rewrite-urls.html'
}
},
rootpath: {
src: ['test/browser/less/rootpath/*.less'],
options: {
Expand All @@ -374,6 +386,14 @@ module.exports = function (grunt) {
outfile: 'tmp/browser/test-runner-rootpath-relative.html'
}
},
rootpathRewriteUrls: {
src: ['test/browser/less/rootpath-rewrite-urls/*.less'],
options: {
helpers: 'test/browser/runner-rootpath-rewrite-urls-options.js',
specs: 'test/browser/runner-rootpath-rewrite-urls-spec.js',
outfile: 'tmp/browser/test-runner-rootpath-rewrite-urls.html'
}
},
production: {
src: ['test/browser/less/production/*.less'],
options: {
Expand Down
18 changes: 16 additions & 2 deletions bin/lessc
Original file line number Diff line number Diff line change
Expand Up @@ -475,9 +475,23 @@ function processPluginQueue() {
options.rootpath = match[2].replace(/\\/g, '/');
}
break;
case 'ru':
case 'relative-urls':
options.relativeUrls = true;
console.warn('The relative-urls option has been replaced by the rewrite-urls option and will be removed.');
options.rewriteUrls = 'all';
break;
case 'ru':
case 'rewrite-urls':
if (match[2] === 'all' || match[2] === 'local') {
options.rewriteUrls = match[2];
} else if (match[2] === 'off') {
options.rewriteUrls = false;
} else if (!match[2]) {
options.rewriteUrls = 'all';
} else {
console.error('Unknown rewrite-urls argument ' + match[2]);
continueProcessing = false;
process.exitCode = 1;
}
break;
case 'sm':
case 'strict-math':
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ task testRhino(type: AllRhinoTests) {
}

task testRhinoBase(type: RhinoTest) {
options = [ '--strict-math=true', '--relative-urls' ]
options = [ '--strict-math=true', '--rewrite-urls' ]
}

task testRhinoDebugAll(type: DebugRhinoTest) {
Expand Down
7 changes: 6 additions & 1 deletion lib/less-browser/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ module.exports = function(window, options) {
if (options.functions) {
less.functions.functionRegistry.addMultiple(options.functions);
}
// Ensure compatibility with deprecated relativeUrls options
if (options.relativeUrls) {
options.rewriteUrls = 'all';
}

var typePattern = /^text\/(x-)?less$/;

Expand Down Expand Up @@ -93,7 +97,8 @@ module.exports = function(window, options) {
currentDirectory: fileManager.getPath(path),
filename: path,
rootFilename: path,
relativeUrls: instanceOptions.relativeUrls};
rewriteUrls: instanceOptions.rewriteUrls
};

newFileInfo.entryPath = newFileInfo.currentDirectory;
newFileInfo.rootpath = instanceOptions.rootpath || newFileInfo.currentDirectory;
Expand Down
2 changes: 1 addition & 1 deletion lib/less-node/image-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module.exports = function(environment) {
function imageSize(functionContext, filePathNode) {
var filePath = filePathNode.value;
var currentFileInfo = functionContext.currentFileInfo;
var currentDirectory = currentFileInfo.relativeUrls ?
var currentDirectory = currentFileInfo.rewriteUrls ?
currentFileInfo.currentDirectory : currentFileInfo.entryPath;

var fragmentStart = filePath.indexOf('#');
Expand Down
3 changes: 2 additions & 1 deletion lib/less-node/lessc-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ var lessc_helper = {
console.log(' in generated CSS file.');
console.log(' -rp, --rootpath=URL Sets rootpath for url rewriting in relative imports and urls');
console.log(' Works with or without the relative-urls option.');
console.log(' -ru, --relative-urls Re-writes relative urls to the base less file.');
console.log(' -ru=, --rewrite-urls= Rewrites URLs to make them relative to the base less file.');
console.log(' all|local|off \'all\' rewrites all URLs, \'local\' just those starting with a \'.\'');
console.log(' -sm=on|off Turns on or off strict math, where in strict mode, math.');
console.log(' --strict-math=on|off Requires brackets. This option may default to on and then');
console.log(' be removed in the future.');
Expand Down
24 changes: 19 additions & 5 deletions lib/less-rhino/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,16 @@ less.Parser.fileLoader = function (file, currentFileInfo, callback, env) {
newFileInfo.entryPath = currentFileInfo.entryPath;
newFileInfo.rootpath = currentFileInfo.rootpath;
newFileInfo.rootFilename = currentFileInfo.rootFilename;
newFileInfo.relativeUrls = currentFileInfo.relativeUrls;
newFileInfo.rewriteUrls = currentFileInfo.rewriteUrls;
} else {
newFileInfo.entryPath = path;
newFileInfo.rootpath = less.rootpath || path;
newFileInfo.rootFilename = href;
newFileInfo.relativeUrls = env.relativeUrls;
newFileInfo.rewriteUrls = env.rewriteUrls;
}

var j = file.lastIndexOf('/');
if (newFileInfo.relativeUrls && !/^(?:[a-z-]+:|\/)/.test(file) && j != -1) {
if (newFileInfo.rewriteUrls && !/^(?:[a-z-]+:|\/)/.test(file) && j != -1) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we need to change that part. However, for me it looks like less.Parser.fileLoader is not used anywhere. Is that file even up-to-date?

Copy link
Member

Choose a reason for hiding this comment

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

No, less-rhino should have been pulled in 3.0, but I just wasn't sure. I doubt it even still works. Ideally, PR #2985 should be re-written and pulled into its own repo with less as a dependency (as in https://github.com/less/less-java), and then less-rhino should be removed as a sub-folder.

Long and short, there aren't any active Java maintainers in the Less core team, so we really can't manage it within this repo.

var relativeSubDirectory = file.slice(0, j + 1);
newFileInfo.rootpath = newFileInfo.rootpath + relativeSubDirectory; // append (sub|sup) directory path of imported file
}
Expand Down Expand Up @@ -307,9 +307,23 @@ function writeFile(filename, content) {
options.rootpath = match[2].replace(/\\/g, '/');
}
break;
case 'ru':
case 'relative-urls':
options.relativeUrls = true;
print('The relative-urls option has been replaced by the rewrite-urls option and will be removed.');
options.rewriteUrls = 'all';
break;
case 'ru':
case 'rewrite-urls':
if (match[2] === 'all' || match[2] === 'local') {
options.rewriteUrls = match[2];
} if (match[2] === 'off') {
options.rewriteUrls = false;
} else if (!match[2]) {
options.rewriteUrls = 'all';
} else {
print('Unknown rewrite-urls argument ' + match[2]);
continueProcessing = false;
currentErrorcode = 1;
}
break;
case 'sm':
case 'strict-math':
Expand Down
42 changes: 35 additions & 7 deletions lib/less/contexts.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var copyFromOriginal = function copyFromOriginal(original, destination, properti
var parseCopyProperties = [
// options
'paths', // option - unmodified - paths to search for imports on
'relativeUrls', // option - whether to adjust URL's to be relative
'rewriteUrls', // option - whether to adjust URL's to be relative
'rootpath', // option - rootpath to append to URL's
'strictImports', // option -
'insecure', // option - whether to allow imports from insecure ssl hosts
Expand Down Expand Up @@ -50,7 +50,8 @@ var evalCopyProperties = [
'urlArgs', // whether to add args into url tokens
'javascriptEnabled', // option - whether Inline JavaScript is enabled. if undefined, defaults to false
'pluginManager', // Used as the plugin manager for the session
'importantScope' // used to bubble up !important statements
'importantScope', // used to bubble up !important statements
'rewriteUrls' // option - whether to adjust URL's to be relative
];

contexts.Eval = function(options, frames) {
Expand Down Expand Up @@ -97,17 +98,36 @@ contexts.Eval.prototype.isMathOn = function () {
return this.strictMath ? (this.parensStack && this.parensStack.length) : true;
};

contexts.Eval.prototype.isPathRelative = function (path) {
return !/^(?:[a-z-]+:|\/|#)/i.test(path);
contexts.Eval.prototype.pathRequiresRewrite = function (path) {
var isRelative = this.rewriteUrls === 'local' ? isPathLocalRelative : isPathRelative;

return isRelative(path);
};

contexts.Eval.prototype.normalizePath = function( path ) {
contexts.Eval.prototype.rewritePath = function (path, rootpath) {
var newPath;

rootpath = rootpath || '';
newPath = this.normalizePath(rootpath + path);

// If a path was explicit relative and the rootpath was not an absolute path
// we must ensure that the new path is also explicit relative.
if (isPathLocalRelative(path) &&
isPathRelative(rootpath) &&
isPathLocalRelative(newPath) === false) {
newPath = './' + newPath;
}

return newPath;
};

contexts.Eval.prototype.normalizePath = function (path) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes in normalizePath are just code style issues

var
segments = path.split('/').reverse(),
segment;

path = [];
while (segments.length !== 0 ) {
while (segments.length !== 0) {
segment = segments.pop();
switch ( segment ) {
case '.':
Expand All @@ -120,12 +140,20 @@ contexts.Eval.prototype.normalizePath = function( path ) {
}
break;
default:
path.push( segment );
path.push(segment);
break;
}
}

return path.join('/');
};

function isPathRelative(path) {
return !/^(?:[a-z-]+:|\/|#)/i.test(path);
}

function isPathLocalRelative(path) {
return path.charAt(0) === '.';
}

// todo - do the same for the toCSS ?
2 changes: 1 addition & 1 deletion lib/less/default-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module.exports = function() {
* that references an image, exactly the same URL will be output in the css.
* This option allows you to re-write URL's in imported files so that the
* URL is always relative to the base imported file */
relativeUrls: false,
rewriteUrls: false,

/* Compatibility with IE8. Used for limiting data-uri length */
ieCompat: false, // true until 3.0
Expand Down
2 changes: 1 addition & 1 deletion lib/less/functions/data-uri.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module.exports = function(environment) {
var mimetype = mimetypeNode && mimetypeNode.value;
var filePath = filePathNode.value;
var currentFileInfo = this.currentFileInfo;
var currentDirectory = currentFileInfo.relativeUrls ?
var currentDirectory = currentFileInfo.rewriteUrls ?
currentFileInfo.currentDirectory : currentFileInfo.entryPath;

var fragmentStart = filePath.indexOf('#');
Expand Down
6 changes: 3 additions & 3 deletions lib/less/import-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var contexts = require('./contexts'),
module.exports = function(environment) {

// FileInfo = {
// 'relativeUrls' - option - whether to adjust URL's to be relative
// 'rewriteUrls' - option - whether to adjust URL's to be relative
// 'filename' - full resolved filename of current file
// 'rootpath' - path to append to normal URLs for this node
// 'currentDirectory' - path to the current file, absolute
Expand Down Expand Up @@ -65,7 +65,7 @@ module.exports = function(environment) {
};

var newFileInfo = {
relativeUrls: this.context.relativeUrls,
rewriteUrls: this.context.rewriteUrls,
entryPath: currentFileInfo.entryPath,
rootpath: currentFileInfo.rootpath,
rootFilename: currentFileInfo.rootFilename
Expand All @@ -92,7 +92,7 @@ module.exports = function(environment) {
// - If path of imported file is '../mixins.less' and rootpath is 'less/',
// then rootpath should become 'less/../'
newFileInfo.currentDirectory = fileManager.getPath(resolvedFilename);
if (newFileInfo.relativeUrls) {
if (newFileInfo.rewriteUrls) {
newFileInfo.rootpath = fileManager.join(
(importManager.context.rootpath || ''),
fileManager.pathDiff(newFileInfo.currentDirectory, newFileInfo.entryPath));
Expand Down
2 changes: 1 addition & 1 deletion lib/less/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ module.exports = function(environment, ParseTree, ImportManager) {
var entryPath = filename.replace(/[^\/\\]*$/, '');
rootFileInfo = {
filename: filename,
relativeUrls: context.relativeUrls,
rewriteUrls: context.rewriteUrls,
rootpath: context.rootpath || '',
currentDirectory: entryPath,
entryPath: entryPath,
Expand Down
17 changes: 9 additions & 8 deletions lib/less/tree/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,18 @@ Import.prototype.evalForImport = function (context) {
};
Import.prototype.evalPath = function (context) {
var path = this.path.eval(context);
var rootpath = this._fileInfo && this._fileInfo.rootpath;
var fileInfo = this._fileInfo;

if (!(path instanceof URL)) {
if (rootpath) {
var pathValue = path.value;
// Add the base path if the import is relative
if (pathValue && context.isPathRelative(pathValue)) {
path.value = rootpath + pathValue;
}
// Add the rootpath if the URL requires a rewrite
var pathValue = path.value;
if (fileInfo &&
pathValue &&
context.pathRequiresRewrite(pathValue)) {
path.value = context.rewritePath(pathValue, fileInfo.rootpath);
} else {
path.value = context.normalizePath(path.value);
}
path.value = context.normalizePath(path.value);
}

return path;
Expand Down
20 changes: 12 additions & 8 deletions lib/less/tree/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,19 @@ URL.prototype.eval = function (context) {
rootpath;

if (!this.isEvald) {
// Add the base path if the URL is relative
// Add the rootpath if the URL requires a rewrite
rootpath = this.fileInfo() && this.fileInfo().rootpath;
if (rootpath &&
if (typeof rootpath === 'string' &&
typeof val.value === 'string' &&
context.isPathRelative(val.value)) {

context.pathRequiresRewrite(val.value)) {
if (!val.quote) {
rootpath = rootpath.replace(/[\(\)'"\s]/g, function(match) { return '\\' + match; });
rootpath = escapePath(rootpath);
}
val.value = rootpath + val.value;
val.value = context.rewritePath(val.value, rootpath);
} else {
val.value = context.normalizePath(val.value);
}

val.value = context.normalizePath(val.value);

// Add url args if enabled
if (context.urlArgs) {
if (!val.value.match(/^\s*data:/)) {
Expand All @@ -51,4 +50,9 @@ URL.prototype.eval = function (context) {

return new URL(val, this.getIndex(), this.fileInfo(), true);
};

function escapePath(path) {
return path.replace(/[\(\)'"\s]/g, function(match) { return '\\' + match; });
}

module.exports = URL;
Loading