Docserver: Fix 3 issues relating to finding API references in sample files,

chrome/common/extensions/docs/server2/samples_data_source.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import hashlib
 import json
 import logging
 import posixpath
 import re
 import traceback
@@ skipping 38 matching lines @@
     def Create(self, request):
       '''Returns a new SamplesDataSource bound to |request|.
       '''
       return SamplesDataSource(self._extensions_cache,
                                self._apps_cache,
                                self._extension_samples_path,
                                self._base_path,
                                request)
 
     def _GetAPIItems(self, js_file):
-      chrome_regex = '(chrome\.[a-zA-Z0-9\.]+)'
+      chrome_regex = r'(chrome\.[a-zA-Z0-9\.]+)'
       calls = set(re.findall(chrome_regex, js_file))
       # Find APIs that have been assigned into variables.
-      assigned_vars = dict(re.findall('var\s*([^\s]+)\s*=\s*%s;' % chrome_regex,
-                                      js_file))
+      assigned_vars = dict(
+          re.findall(r'var\s*([^\s]+)\s*=\s*%s;' % chrome_regex, js_file))
       # Replace the variable name with the full API name.
       for var_name, value in assigned_vars.iteritems():

[Jeffrey Yasskin, 2013/11/01 18:00:24]

You might be able to make this loop more efficient by joining all the
|var_name|s into a single regex, and then using a function argument to re.sub()
to pick the right value to substitute. Or the function calls might make it less
efficient than copying the string N times. But given the size of the samples,
probably the function would be faster.

-        js_file = js_file.replace(var_name, value)
+        js_file = re.compile(r'\b%s\b' % var_name).sub(value, js_file)

[not at google - send to devlin, 2013/11/01 17:12:04]

this is the most important change.

[Jeffrey Yasskin, 2013/11/01 18:00:24]

Instead of re.compile(variable_string).sub(...), use re.sub(variable_string,
value, js_file)
http://docs.python.org/2/library/re.html#re.sub

[Jeffrey Yasskin, 2013/11/01 18:00:24]

You probably want re.escape(var_name) in case it contains '\' or other special
characters. Otherwise assert that it's purely alphanumeric, or guarantee it with
something stronger than [^\s]+ in the assign_vars re above.

       return calls.union(re.findall(chrome_regex, js_file))

[Jeffrey Yasskin, 2013/11/01 18:00:24]

Wait a second, you're just trying to find the APIs used in the file, so why go
to the expense of replacing things at all? You can use re.finditer to find each
use of an API that's been assigned into a variable, and then
re.compile(r'\.[a-zA-Z0-9\.]+').match(js_file, var_match.end()) to find the
suffix. (share the suffix re across iterations, of course.)

[not at google - send to devlin, 2013/11/04 21:51:01]

I got a little bit lost in your comments there + the deeply confusing re
library, but, how does this look? I re-wrote it rather than trying to patch it.

I've removed the actual replacement and just done plain searching; although now
it's O(T + T*V) where T is the length of the text and V is the number of
variable matches. That's probably fine since there will most likely be V=0 which
should offset the cost of doing text replacement; I presume.

 
     def _GetDataFromManifest(self, path, file_system):
       manifest = file_system.ReadSingle(path + '/manifest.json').Get()
       try:
         manifest_json = json.loads(json_comment_eater.Nom(manifest))
       except ValueError as e:
         logging.error('Error parsing manifest.json for %s: %s' % (path, e))
         return None
       l10n_data = {
         'name': manifest_json.get('name', ''),
@@ skipping 152 matching lines @@
       else:
         dict_['id'] = self._GetSampleId(name)
         return_list.append(dict_)
     return return_list
 
   def get(self, key):
     return {
       'apps': lambda: self._CreateSamplesDict('apps'),
       'extensions': lambda: self._CreateSamplesDict('extensions')
     }.get(key, lambda: {})()