Trailing semicolon (#2962)

* Only put trailing semicolon when merging style attributes This is a first draft that tries to maintain the same code behavior without worrying too much about the code quality * Update tests so they reflect the new code behavior All style attributes that received a String won't generate a trailing semicolon * Improve merge trailing semicolon code Check if String is not empty by checking if it its true ('' is false) Get last character of the string using slice(-1) Add semicolon if the last string character is not a semicolon * Use array style indexing instead of `.slice` to get last character
pugjs · Feb 19, 2018 · aebb187 · aebb187
1 parent 8248ba8
commit aebb187
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 16 deletions.
diff --git a/packages/pug-runtime/index.js b/packages/pug-runtime/index.js
@@ -30,7 +30,9 @@ function pug_merge(a, b) {
       a[key] = (Array.isArray(valA) ? valA : [valA]).concat(b[key] || []);
     } else if (key === 'style') {
       var valA = pug_style(a[key]);
+      valA = valA && valA[valA.length - 1] !== ';' ? valA + ';' : valA;
       var valB = pug_style(b[key]);
+      valB = valB && valB[valB.length - 1] !== ';' ? valB + ';' : valB;
       a[key] = valA + valB;
     } else {
       a[key] = b[key];
@@ -109,10 +111,7 @@ function pug_style(val) {
     }
     return out;
   } else {
-    val += '';
-    if (val[val.length - 1] !== ';') 
-      return val + ';';
-    return val;
+    return val + '';
   }
 };
 

diff --git a/packages/pug-runtime/test/index.test.js b/packages/pug-runtime/test/index.test.js
@@ -132,7 +132,7 @@ addTest('merge', function (merge) {
 addTest('style', function (style) {
   expect(style(null)).toBe('');
   expect(style('')).toBe('');
-  expect(style('foo: bar')).toBe('foo: bar;');
+  expect(style('foo: bar')).toBe('foo: bar');
   expect(style('foo: bar;')).toBe('foo: bar;');
   expect(style({foo: 'bar'})).toBe('foo:bar;');
   expect(style({foo: 'bar', baz: 'bash'})).toBe('foo:bar;baz:bash;');

diff --git a/packages/pug/test/cases-es2015/attr.html b/packages/pug/test/cases-es2015/attr.html
@@ -1 +1 @@
-<div class="avatar-div" style="background-image: url(https://www.gravatar.com/avatar/219b77f9d21de75e81851b6b886057c7);"></div>
+<div class="avatar-div" style="background-image: url(https://www.gravatar.com/avatar/219b77f9d21de75e81851b6b886057c7)"></div>
diff --git a/packages/pug/test/cases/styles.html b/packages/pug/test/cases/styles.html
@@ -7,11 +7,11 @@
     </style>
   </head>
   <body>
+    <div style="color:red;background:green"></div>
     <div style="color:red;background:green;"></div>
+    <div style="color:red;background:green"></div>
     <div style="color:red;background:green;"></div>
-    <div style="color:red;background:green;"></div>
-    <div style="color:red;background:green;"></div>
-        <div style="color:red;background:green;"></div>
+        <div style="color:red;background:green"></div>
         <div style="color:red;background:green;"></div>
     <div style="color:red;background:green;"></div>
     <div style="color:red;background:green;"></div>

diff --git a/packages/pug/test/output-es2015/attr.html b/packages/pug/test/output-es2015/attr.html
@@ -1,2 +1,2 @@
 
-<div class="avatar-div" style="background-image: url(https://www.gravatar.com/avatar/219b77f9d21de75e81851b6b886057c7);"></div>
+<div class="avatar-div" style="background-image: url(https://www.gravatar.com/avatar/219b77f9d21de75e81851b6b886057c7)"></div>
diff --git a/packages/pug/test/pug.test.js b/packages/pug/test/pug.test.js
@@ -371,8 +371,8 @@ describe('pug', function(){
       assert.equal('<a href="http://google.com" title="Some : weird = title"></a>', pug.render('a(href= "http://google.com", title= "Some : weird = title")'));
       assert.equal('<label for="name"></label>', pug.render('label(for="name")'));
       assert.equal('<meta name="viewport" content="width=device-width"/>', pug.render("meta(name= 'viewport', content='width=device-width')"), 'Test attrs that contain attr separators');
-      assert.equal('<div style="color= white;"></div>', pug.render("div(style='color= white')"));
-      assert.equal('<div style="color: white;"></div>', pug.render("div(style='color: white')"));
+      assert.equal('<div style="color= white"></div>', pug.render("div(style='color= white')"));
+      assert.equal('<div style="color: white"></div>', pug.render("div(style='color: white')"));
       assert.equal('<p class="foo"></p>', pug.render("p('class'='foo')"), 'Test keys with single quotes');
       assert.equal('<p class="foo"></p>', pug.render("p(\"class\"= 'foo')"), 'Test keys with double quotes');
 
@@ -389,10 +389,10 @@ describe('pug', function(){
 
       assert.equal('<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1"/>', pug.render('meta(http-equiv="X-UA-Compatible", content="IE=edge,chrome=1")'));
 
-      assert.equal('<div style="background: url(/images/test.png);">Foo</div>', pug.render("div(style= 'background: url(/images/test.png)') Foo"));
-      assert.equal('<div style="background = url(/images/test.png);">Foo</div>', pug.render("div(style= 'background = url(/images/test.png)') Foo"));
-      assert.equal('<div style="foo;">Foo</div>', pug.render("div(style= ['foo', 'bar'][0]) Foo"));
-      assert.equal('<div style="bar;">Foo</div>', pug.render("div(style= { foo: 'bar', baz: 'raz' }['foo']) Foo"));
+      assert.equal('<div style="background: url(/images/test.png)">Foo</div>', pug.render("div(style= 'background: url(/images/test.png)') Foo"));
+      assert.equal('<div style="background = url(/images/test.png)">Foo</div>', pug.render("div(style= 'background = url(/images/test.png)') Foo"));
+      assert.equal('<div style="foo">Foo</div>', pug.render("div(style= ['foo', 'bar'][0]) Foo"));
+      assert.equal('<div style="bar">Foo</div>', pug.render("div(style= { foo: 'bar', baz: 'raz' }['foo']) Foo"));
       assert.equal('<a href="def">Foo</a>', pug.render("a(href='abcdefg'.substr(3,3)) Foo"));
       assert.equal('<a href="def">Foo</a>', pug.render("a(href={test: 'abcdefg'}.test.substr(3,3)) Foo"));
       assert.equal('<a href="def">Foo</a>', pug.render("a(href={test: 'abcdefg'}.test.substr(3,[0,3][1])) Foo"));