[dmd-internals] Pulls with whitespace changes

Iain Buclaw ibuclaw at gdcproject.org
Wed Jan 29 00:10:22 PST 2014


I've said this often in pulls, but I still see it often enough - both
in pulls that I look over before or after they've been merge - so
would like to make an (informal formal) request in here.

Can people please try to keep whitespace changes / code reformatting
out of pull requests that fix real issues?  They serve nothing but
clutter up the pull making it difficult to review, and may hide or
introduce bugs in the process.

For example:
---
@@ -301,15 +614,19 @@ class CppMangleVisitor : public Visitor
-        if (substitute(t))
-            return;
+        if (substitute(t)) return;
         buf.writeByte('F');
         if (t->linkage == LINKc)
             buf.writeByte('Y');
-        t->next->accept(this);
+        Type *tn = t->next;
+        if (t->isref)
+            tn  = tn->referenceTo();
+        tn->accept(this);
         argsCppMangle(t->parameters, t->varargs);

         buf.writeByte('E');
         store(t);
+
+
     }

Which both a needless change to have if-return on one line (and IMO
makes the code *look* worse).  And also adds two spurious empty lines
at the end of the function.

Another example:
---
@@ -1698,7 +1699,8 @@ EnumDeclaration *Parser::parseEnum()
             if (token.value == TOKrcurly)
                 ;
             else
-            {   addComment(em, comment);
+            {
+                addComment(em, comment);
                 comment = NULL;
                 check(TOKcomma);
             }

Which reformats code that has no relation to the bug being fix.

So this is me saying come on! I would like to see some foot put down
on this and request that contributors make such changes as separate
pulls.

Ciao a tutti.
Iain.


More information about the dmd-internals mailing list