gclient_eval: Add more support when adding new variables.

Now we respect comments before the first variable.

Bug: 760633
Change-Id: Ibe60d719429c033415bfb1c99942c9d04601d967
Reviewed-on: https://chromium-review.googlesource.com/998683
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Michael Moss <mmoss@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

gclient_eval.py

@@ -542,13 +542,15 @@ def AddVar(gclient_dict, var_name, value):
   if not gclient_dict['vars']:
     raise ValueError('vars dict is empty. This is not yet supported.')
 
-  # We will attempt to add the var right before the first var.
-  node = gclient_dict.GetNode('vars').keys[0]
+  # We will attempt to add the var right after 'vars = {'.
+  node = gclient_dict.GetNode('vars')
   if node is None:
     raise ValueError(
         "The vars dict has no formatting information." % var_name)
-  line = node.lineno
-  col = node.col_offset
+  line = node.lineno + 1
+
+  # We will try to match the new var's indentation to the next variable.
+  col = node.keys[0].col_offset
 
   # We use a minimal Python dictionary, so that ast can parse it.
   var_content = '{\n%s"%s": "%s",\n}' % (' ' * col, var_name, value)

recipes/trigger_recipe_roller.txt

@@ -1,4 +1,4 @@
-No-op file. Edit this to kick recipes.
+No-op file. Edit this to kick recipes. (Did they do something wrong?)
 
 This is a beginning of a story in this silly file.
 Once upon a time, a budding web browser dev team needed a CI system.

tests/gclient_eval_unittest.py

@@ -237,22 +237,25 @@ class EvaluateConditionTest(unittest.TestCase):
 
 
 class AddVarTest(unittest.TestCase):
+  def assert_adds_var(self, before, after):
+    local_scope = gclient_eval.Exec('\n'.join(before))
+    gclient_eval.AddVar(local_scope, 'baz', 'lemur')
+    results = gclient_eval.RenderDEPSFile(local_scope)
+    self.assertEqual(results, '\n'.join(after))
+
   def test_adds_var(self):
-    local_scope = gclient_eval.Exec('\n'.join([
+    before = [
         'vars = {',
         '  "foo": "bar",',
         '}',
-    ]))
-
-    gclient_eval.AddVar(local_scope, 'baz', 'lemur')
-    result = gclient_eval.RenderDEPSFile(local_scope)
-
-    self.assertEqual(result, '\n'.join([
+    ]
+    after = [
         'vars = {',
         '  "baz": "lemur",',
         '  "foo": "bar",',
         '}',
-    ]))
+    ]
+    self.assert_adds_var(before, after)
 
   def test_adds_var_twice(self):
     local_scope = gclient_eval.Exec('\n'.join([
@@ -274,39 +277,38 @@ class AddVarTest(unittest.TestCase):
     ]))
 
   def test_preserves_formatting(self):
-    local_scope = gclient_eval.Exec('\n'.join([
+    before = [
         '# Copyright stuff',
         '# some initial comments',
         '',
         'vars = { ',
+        '  # Some comments.',
         '  "foo": "bar",',
-        '  # Some commets.',
+        '',
         '  # More comments.',
         '  # Even more comments.',
         '  "v8_revision":   ',
         '       "deadbeef",',
         ' # Someone formatted this wrong',
         '}',
-    ]))
-
-    gclient_eval.AddVar(local_scope, 'baz', 'lemur')
-    result = gclient_eval.RenderDEPSFile(local_scope)
-
-    self.assertEqual(result, '\n'.join([
+    ]
+    after = [
         '# Copyright stuff',
         '# some initial comments',
         '',
         'vars = { ',
         '  "baz": "lemur",',
+        '  # Some comments.',
         '  "foo": "bar",',
-        '  # Some commets.',
+        '',
         '  # More comments.',
         '  # Even more comments.',
         '  "v8_revision":   ',
         '       "deadbeef",',
         ' # Someone formatted this wrong',
         '}',
-    ]))
+    ]
+    self.assert_adds_var(before, after)
 
 
 class SetVarTest(unittest.TestCase):