Fixes #525 - restores `inherit`-based merging.

Why:

* Merging longhands into shorthand when `inherit` is present is possible
  and this commit restores this functionality;
* it was previously removed after big level 2 optimizations refactoring
  in clean-css 3.2.

diff --git a/History.md b/History.md
index d23f801..76830cb 100644 (file)
--- a/History.md
+++ b/History.md
@@ -2,6 +2,7 @@
 ==================
 
 * Fixed issue [#254](https://github.com/jakubpawlowicz/clean-css/issues/254) - adds `font` property optimizer.
+* Fixed issue [#525](https://github.com/jakubpawlowicz/clean-css/issues/525) - restores `inherit`-based merging.
 * Fixed issue [#755](https://github.com/jakubpawlowicz/clean-css/issues/755) - adds custom handling of remote requests.
 * Fixed issue [#860](https://github.com/jakubpawlowicz/clean-css/issues/860) - adds `animation` property optimizer.
 * Fixed issue [#862](https://github.com/jakubpawlowicz/clean-css/issues/862) - allows removing unused at rules.

diff --git a/lib/optimizer/level-2/compactable.js b/lib/optimizer/level-2/compactable.js
index 097b6cf..b076a0d 100644 (file)
--- a/lib/optimizer/level-2/compactable.js
+++ b/lib/optimizer/level-2/compactable.js
@@ -359,6 +359,7 @@ var compactable = {
       'border-width'
     ],
     defaultValue: 'medium',
+    oppositeTo: 'border-top-width',
     shortestValue: '0'
   },
   'border-collapse': {
@@ -424,6 +425,7 @@ var compactable = {
       'border-width'
     ],
     defaultValue: 'medium',
+    oppositeTo: 'border-right-width',
     shortestValue: '0'
   },
   'border-radius': {
@@ -487,6 +489,7 @@ var compactable = {
       'border-width'
     ],
     defaultValue: 'medium',
+    oppositeTo: 'border-left-width',
     shortestValue: '0'
   },
   'border-style': {
@@ -571,6 +574,7 @@ var compactable = {
       'border-width'
     ],
     defaultValue: 'medium',
+    oppositeTo: 'border-bottom-width',
     shortestValue: '0'
   },
   'border-width': {
@@ -741,28 +745,32 @@ var compactable = {
     componentOf: [
       'margin'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'margin-top'
   },
   'margin-left': {
     canOverride: canOverride.generic.unit,
     componentOf: [
       'margin'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'margin-right'
   },
   'margin-right': {
     canOverride: canOverride.generic.unit,
     componentOf: [
       'margin'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'margin-left'
   },
   'margin-top': {
     canOverride: canOverride.generic.unit,
     componentOf: [
       'margin'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'margin-bottom'
   },
   'outline': {
     canOverride: canOverride.generic.components([
@@ -838,28 +846,32 @@ var compactable = {
     componentOf: [
       'padding'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'padding-top'
   },
   'padding-left': {
     canOverride: canOverride.generic.unit,
     componentOf: [
       'padding'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'padding-right'
   },
   'padding-right': {
     canOverride: canOverride.generic.unit,
     componentOf: [
       'padding'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'padding-left'
   },
   'padding-top': {
     canOverride: canOverride.generic.unit,
     componentOf: [
       'padding'
     ],
-    defaultValue: '0'
+    defaultValue: '0',
+    oppositeTo: 'padding-bottom'
   },
   'position': {
     canOverride: canOverride.property.position,

diff --git a/lib/optimizer/level-2/properties/merge-into-shorthands.js b/lib/optimizer/level-2/properties/merge-into-shorthands.js
index a75c4ed..bce4ed8 100644 (file)
--- a/lib/optimizer/level-2/properties/merge-into-shorthands.js
+++ b/lib/optimizer/level-2/properties/merge-into-shorthands.js
@@ -4,11 +4,15 @@ var populateComponents = require('./populate-components');
 
 var compactable = require('../compactable');
 var deepClone = require('../clone').deep;
+var restoreWithComponents = require('../restore-with-components');
 
+var restoreFromOptimizing = require('../../restore-from-optimizing');
 var wrapSingle = require('../../wrap-for-optimizing').single;
 
+var serializeBody = require('../../../writer/one-time').body;
 var Token = require('../../../tokenizer/token');
 
+
 function mixedImportance(components) {
   var important;
 
@@ -22,6 +26,49 @@ function mixedImportance(components) {
   return false;
 }
 
+function overridable(components, name, validator) {
+  var descriptor = compactable[name];
+  var newValuePlaceholder = [
+    Token.PROPERTY,
+    [Token.PROPERTY_NAME, name],
+    [Token.PROPERTY_VALUE, descriptor.defaultValue]
+  ];
+  var newProperty = wrapSingle(newValuePlaceholder);
+  var component;
+  var mayOverride;
+  var i, l;
+
+  populateComponents([newProperty], validator, []);
+
+  for (i = 0, l = descriptor.components.length; i < l; i++) {
+    component = components[descriptor.components[i]];
+    mayOverride = compactable[component.name].canOverride;
+
+    if (!everyValuesPair(mayOverride.bind(null, validator), newProperty.components[i], component)) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+function mixedInherit(components) {
+  var lastValue = null;
+  var currentValue;
+
+  for (var name in components) {
+    currentValue = hasInherit(components[name]);
+
+    if (lastValue !== null && lastValue !== currentValue) {
+      return true;
+    }
+
+    lastValue = currentValue;
+  }
+
+  return false;
+}
+
 function joinMetadata(components, at) {
   var metadata = [];
   var component;
@@ -37,7 +84,180 @@ function joinMetadata(components, at) {
     Array.prototype.push.apply(metadata, componentMetadata);
   }
 
-  return metadata;
+  return metadata.sort(metadataSorter);
+}
+
+function metadataSorter(metadata1, metadata2) {
+  var line1 = metadata1[0];
+  var line2 = metadata2[0];
+  var column1 = metadata1[1];
+  var column2 = metadata2[1];
+
+  if (line1 < line2) {
+    return -1;
+  } else if (line1 === line2) {
+    return column1 < column2 ? -1 : 1;
+  } else {
+    return 1;
+  }
+}
+
+function replaceWithInheritBestFit(properties, candidateComponents, name, validator) {
+  var viaLonghands = buildSequenceWithInheritLonghands(candidateComponents, name, validator);
+  var viaShorthand = buildSequenceWithInheritShorthand(candidateComponents, name, validator);
+  var longhandTokensSequence = viaLonghands[0];
+  var shorthandTokensSequence = viaShorthand[0];
+  var isLonghandsShorter = serializeBody(longhandTokensSequence).length < serializeBody(shorthandTokensSequence).length;
+  var newTokensSequence = isLonghandsShorter ? longhandTokensSequence : shorthandTokensSequence;
+  var newProperty = isLonghandsShorter ? viaLonghands[1] : viaShorthand[1];
+  var newComponents = isLonghandsShorter ? viaLonghands[2] : viaShorthand[2];
+  var all = candidateComponents[Object.keys(candidateComponents)[0]].all;
+  var componentName;
+  var oldComponent;
+  var newComponent;
+  var newToken;
+
+  newProperty.position = all.length;
+  newProperty.shorthand = true;
+  newProperty.dirty = true;
+  newProperty.all = all;
+  newProperty.all.push(newTokensSequence[0]);
+
+  properties.push(newProperty);
+
+  for (componentName in candidateComponents) {
+    oldComponent = candidateComponents[componentName];
+    oldComponent.unused = true;
+
+    if (oldComponent.name in newComponents) {
+      newComponent = newComponents[oldComponent.name];
+      newToken = findTokenIn(newTokensSequence, componentName);
+
+      newComponent.position = all.length;
+      newComponent.all = all;
+      newComponent.all.push(newToken);
+
+      properties.push(newComponent);
+    }
+  }
+}
+
+function buildSequenceWithInheritLonghands(components, name, validator) {
+  var tokensSequence = [];
+  var inheritComponents = {};
+  var nonInheritComponents = {};
+  var descriptor = compactable[name];
+  var shorthandToken = [
+    Token.PROPERTY,
+    [Token.PROPERTY_NAME, name],
+    [Token.PROPERTY_VALUE, descriptor.defaultValue]
+  ];
+  var newProperty = wrapSingle(shorthandToken);
+  var component;
+  var longhandToken;
+  var newComponent;
+  var nameMetadata;
+  var i, l;
+
+  populateComponents([newProperty], validator, []);
+
+  for (i = 0, l = descriptor.components.length; i < l; i++) {
+    component = components[descriptor.components[i]];
+
+    if (hasInherit(component)) {
+      longhandToken = component.all[component.position].slice(0, 2);
+      Array.prototype.push.apply(longhandToken, component.value);
+      tokensSequence.push(longhandToken);
+
+      newComponent = deepClone(component);
+      newComponent.value = inferComponentValue(components, newComponent.name);
+
+      newProperty.components[i] = newComponent;
+      inheritComponents[component.name] = deepClone(component);
+    } else {
+      newComponent = deepClone(component);
+      newComponent.all = component.all;
+      newProperty.components[i] = newComponent;
+
+      nonInheritComponents[component.name] = component;
+    }
+  }
+
+  nameMetadata = joinMetadata(nonInheritComponents, 1);
+  shorthandToken[1].push(nameMetadata);
+
+  restoreFromOptimizing([newProperty], restoreWithComponents);
+
+  shorthandToken = shorthandToken.slice(0, 2);
+  Array.prototype.push.apply(shorthandToken, newProperty.value);
+
+  tokensSequence.unshift(shorthandToken);
+
+  return [tokensSequence, newProperty, inheritComponents];
+}
+
+function inferComponentValue(components, propertyName) {
+  var descriptor = compactable[propertyName];
+
+  if ('oppositeTo' in descriptor) {
+    return components[descriptor.oppositeTo].value;
+  } else {
+    return [[Token.PROPERTY_VALUE, descriptor.defaultValue]];
+  }
+}
+
+function buildSequenceWithInheritShorthand(components, name, validator) {
+  var tokensSequence = [];
+  var inheritComponents = {};
+  var nonInheritComponents = {};
+  var descriptor = compactable[name];
+  var shorthandToken = [
+    Token.PROPERTY,
+    [Token.PROPERTY_NAME, name],
+    [Token.PROPERTY_VALUE, 'inherit']
+  ];
+  var newProperty = wrapSingle(shorthandToken);
+  var component;
+  var longhandToken;
+  var nameMetadata;
+  var valueMetadata;
+  var i, l;
+
+  populateComponents([newProperty], validator, []);
+
+  for (i = 0, l = descriptor.components.length; i < l; i++) {
+    component = components[descriptor.components[i]];
+
+    if (hasInherit(component)) {
+      inheritComponents[component.name] = component;
+    } else {
+      longhandToken = component.all[component.position].slice(0, 2);
+      Array.prototype.push.apply(longhandToken, component.value);
+      tokensSequence.push(longhandToken);
+
+      nonInheritComponents[component.name] = deepClone(component);
+    }
+  }
+
+  nameMetadata = joinMetadata(inheritComponents, 1);
+  shorthandToken[1].push(nameMetadata);
+
+  valueMetadata = joinMetadata(inheritComponents, 2);
+  shorthandToken[2].push(valueMetadata);
+
+  tokensSequence.unshift(shorthandToken);
+
+  return [tokensSequence, newProperty, nonInheritComponents];
+}
+
+function findTokenIn(tokens, componentName) {
+  var i, l;
+
+  for (i = 0, l = tokens.length; i < l; i++) {
+    if (tokens[i][1][1] == componentName) {
+      return tokens[i];
+    }
+  }
 }
 
 function replaceWithShorthand(properties, candidateComponents, name, validator) {
@@ -49,7 +269,6 @@ function replaceWithShorthand(properties, candidateComponents, name, validator)
     [Token.PROPERTY_NAME, name],
     [Token.PROPERTY_VALUE, descriptor.defaultValue]
   ];
-  var mayOverride;
   var all;
 
   var newProperty = wrapSingle(newValuePlaceholder);
@@ -61,13 +280,6 @@ function replaceWithShorthand(properties, candidateComponents, name, validator)
   for (var i = 0, l = descriptor.components.length; i < l; i++) {
     var component = candidateComponents[descriptor.components[i]];
 
-    if (hasInherit(component))
-      return;
-
-    mayOverride = compactable[component.name].canOverride;
-    if (!everyValuesPair(mayOverride.bind(null, validator), newProperty.components[i], component))
-      return;
-
     newProperty.components[i] = deepClone(component);
     newProperty.important = component.important;
 
@@ -108,7 +320,14 @@ function invalidateOrCompact(properties, position, candidates, validator) {
     if (mixedImportance(candidateComponents))
       continue;
 
-    replaceWithShorthand(properties, candidateComponents, name, validator);
+    if (!overridable(candidateComponents, name, validator))
+      continue;
+
+    if (mixedInherit(candidateComponents)) {
+      replaceWithInheritBestFit(properties, candidateComponents, name, validator);
+    } else {
+      replaceWithShorthand(properties, candidateComponents, name, validator);
+    }
   }
 }
 

diff --git a/test/optimizer/level-2/properties/merge-into-shorthands-test.js b/test/optimizer/level-2/properties/merge-into-shorthands-test.js
index 03f3e11..f92c789 100644 (file)
--- a/test/optimizer/level-2/properties/merge-into-shorthands-test.js
+++ b/test/optimizer/level-2/properties/merge-into-shorthands-test.js
@@ -506,12 +506,38 @@ vows.describe(optimizeProperties)
         ]);
       }
     },
-    'with inherit - pending #525': {
+    'with inherit - one': {
       'topic': function () {
         return _optimize('a{padding-top:10px;padding-left:5px;padding-bottom:3px;padding-right:inherit}');
       },
       'into': function (properties) {
         assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'padding', [[1, 2, undefined], [1, 19, undefined], [1, 36, undefined]]],
+            ['property-value', '10px', [[1, 14, undefined]]],
+            ['property-value', '5px', [[1, 32, undefined]]],
+            ['property-value', '3px', [[1, 51, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'padding-right', [[1, 55, undefined]]],
+            ['property-value', 'inherit', [[1, 69, undefined]]]
+          ]
+        ]);
+      }
+    },
+    'with inherit - two': {
+      'topic': function () {
+        return _optimize('a{padding-top:10px;padding-left:5px;padding-bottom:inherit;padding-right:inherit}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'padding', [[1, 36, undefined], [1, 59, undefined]]],
+            ['property-value', 'inherit', [[1, 51, undefined], [1, 73, undefined]]]
+          ],
           [
             'property',
             ['property-name', 'padding-top', [[1, 2, undefined]]],
@@ -521,16 +547,59 @@ vows.describe(optimizeProperties)
             'property',
             ['property-name', 'padding-left', [[1, 19, undefined]]],
             ['property-value', '5px', [[1, 32, undefined]]]
+          ]
+        ]);
+      }
+    },
+    'with inherit - three': {
+      'topic': function () {
+        return _optimize('a{padding-top:inherit;padding-left:5px;padding-bottom:inherit;padding-right:inherit}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'padding', [[1, 2, undefined], [1, 39, undefined], [1, 62, undefined]]],
+            ['property-value', 'inherit', [[1, 14, undefined], [1, 54, undefined], [1, 76, undefined]]]
           ],
           [
             'property',
-            ['property-name', 'padding-bottom', [[1, 36, undefined]]],
-            ['property-value', '3px', [[1, 51, undefined]]]
+            ['property-name', 'padding-left', [[1, 22, undefined]]],
+            ['property-value', '5px', [[1, 35, undefined]]]
+          ]
+        ]);
+      }
+    },
+    'with inherit - four': {
+      'topic': function () {
+        return _optimize('a{padding-top:inherit;padding-left:inherit;padding-bottom:inherit;padding-right:inherit}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'padding', [[1, 2, undefined], [1, 22, undefined], [1, 43, undefined], [1, 66, undefined]]],
+            ['property-value', 'inherit', [[1, 14, undefined]]]
+          ]
+        ]);
+      }
+    },
+    'with inherit - outline': {
+      'topic': function () {
+        return _optimize('.block{outline-width:inherit;outline-style:solid;outline-color:red}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'outline', [[1, 29, undefined], [1, 49, undefined]]],
+            ['property-value', 'red', [[1, 63, undefined]]],
+            ['property-value', 'solid', [[1, 43, undefined]]]
           ],
           [
             'property',
-            ['property-name', 'padding-right', [[1, 55, undefined]]],
-            ['property-value', 'inherit', [[1, 69, undefined]]]
+            ['property-name', 'outline-width', [[1, 7, undefined]]],
+            ['property-value', 'inherit', [[1, 21, undefined]]],
           ]
         ]);
       }