From 4422e7789c257a06e6b8b0a010a3576986c9752e Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Thu, 18 Dec 2014 00:26:22 +0000 Subject: [PATCH] Fixes #398 - restoring important comments. * Should be rather done on tokenization step. To be improved in 3.1. --- History.md | 1 + lib/properties/optimizer.js | 4 ++-- lib/selectors/source-map-stringifier.js | 16 ++++++++++++++-- lib/selectors/stringifier.js | 14 ++++++++++++-- test/integration-test.js | 8 ++++++++ test/source-map-test.js | 14 ++++++++++++++ 6 files changed, 51 insertions(+), 6 deletions(-) diff --git a/History.md b/History.md index 91578be8..319f0e93 100644 --- a/History.md +++ b/History.md @@ -23,6 +23,7 @@ * Fixed issue [#363](https://github.com/GoalSmashers/clean-css/issues/363) - `rem` units overriding `px`. * Fixed issue [#373](https://github.com/GoalSmashers/clean-css/issues/373) - proper background shorthand merging. * Fixed issue [#395](https://github.com/GoalSmashers/clean-css/issues/395) - unescaped brackets in data URIs. +* Fixed issue [#398](https://github.com/GoalSmashers/clean-css/issues/398) - restoring important comments. * Fixed issue [#400](https://github.com/GoalSmashers/clean-css/issues/400) - API to accept an array of filenames. * Fixed issue [#403](https://github.com/GoalSmashers/clean-css/issues/403) - tracking input files in source maps. * Fixed issue [#404](https://github.com/GoalSmashers/clean-css/issues/404) - no state sharing in API. diff --git a/lib/properties/optimizer.js b/lib/properties/optimizer.js index 4ae72c4e..22e604dc 100644 --- a/lib/properties/optimizer.js +++ b/lib/properties/optimizer.js @@ -238,8 +238,8 @@ module.exports = function Optimizer(options, context) { eligibleForCompacting = true; // FIXME: the check should be gone with #396 - var property = tokens[i][0].indexOf('__ESCAPED_') === 0 ? - tokens[i][0] : + var property = !tokens[i][0] && tokens[i][1].indexOf('__ESCAPED_') === 0 ? + tokens[i][1] : tokens[i][0] + ':' + tokens[i][1]; tokenized.push({ value: property, metadata: tokens[i][4] }); list.push(property); diff --git a/lib/selectors/source-map-stringifier.js b/lib/selectors/source-map-stringifier.js index ddffc2e5..f9a7d3ed 100644 --- a/lib/selectors/source-map-stringifier.js +++ b/lib/selectors/source-map-stringifier.js @@ -38,9 +38,21 @@ Rebuilder.prototype.relativePathResolver = function (sourcePath, sourceRelativeT }; Rebuilder.prototype.rebuildValue = function (list, separator) { + var lastEscaped = false; + for (var i = 0, l = list.length; i < l; i++) { - this.store(list[i]); - this.store(i < l - 1 ? separator : ''); + var el = list[i]; + + if (el.value.indexOf('__ESCAPED_') === 0) { + if (!lastEscaped) + this.output.pop(); + this.store(el); + lastEscaped = true; + } else { + this.store(el); + this.store(i < l - 1 ? separator : ''); + lastEscaped = false; + } } }; diff --git a/lib/selectors/stringifier.js b/lib/selectors/stringifier.js index 685a0cf2..baa0010d 100644 --- a/lib/selectors/stringifier.js +++ b/lib/selectors/stringifier.js @@ -7,9 +7,19 @@ function Stringifier(options, restoreCallback) { function valueRebuilder(list, separator) { var merged = ''; + var lastEscaped = false; - for (var i = 0, l = list.length; i < l; i++) - merged += list[i].value + (i < l - 1 ? separator : ''); + for (var i = 0, l = list.length; i < l; i++) { + var el = list[i]; + + if (el.value.indexOf('__ESCAPED_') === 0) { + merged = (lastEscaped ? merged : merged.substring(0, merged.length - 1)) + el.value; + lastEscaped = true; + } else { + merged += list[i].value + (i < l - 1 ? separator : ''); + lastEscaped = false; + } + } return merged; } diff --git a/test/integration-test.js b/test/integration-test.js index 1194761a..7f6fff70 100644 --- a/test/integration-test.js +++ b/test/integration-test.js @@ -295,6 +295,14 @@ vows.describe('integration tests').addBatch({ 'with quote marks': [ '/*"*//* */', '' + ], + 'important after value': [ + 'div{color:red!important;/*!comment*/}', + 'div{color:red!important/*!comment*/}' + ], + 'two important after value': [ + 'div{color:red!important;/*!1*//*!2*/}', + 'div{color:red!important/*!1*//*!2*/}' ] }), 'escaping': cssContext({ diff --git a/test/source-map-test.js b/test/source-map-test.js index f9fa76c9..5359d54f 100644 --- a/test/source-map-test.js +++ b/test/source-map-test.js @@ -663,4 +663,18 @@ vows.describe('source-map') } } }) + .addBatch({ + 'important comment after a property': { + 'topic': new CleanCSS({ sourceMap: true }).minify('div { color: #f00 !important; /*!comment*/ }'), + 'has right output': function (errors, minified) { + assert.equal(minified.styles, 'div{color:red!important/*!comment*/}'); + } + }, + 'important comments after a property': { + 'topic': new CleanCSS({ sourceMap: true }).minify('div { color: #f00 !important; /*!1*//*!2*/ }'), + 'has right output': function (errors, minified) { + assert.equal(minified.styles, 'div{color:red!important/*!1*//*!2*/}'); + } + } + }) .export(module); -- 2.34.1