Merge lp:~widelands-dev/widelands/automate_clang-format into lp:widelands

[GunChleoc]

Moved utils/fix_lua_tabs.py to utils/fix_formatting.py and added calls to clang-format any pyformat.

I think it would be a good idea to automate clang-format - it is starting to cost me real time when I work on a branch and trunk hasn't been formatted properly.

Can this be added to the bunnybot merge command once we have this in trunk? I'm thinking bzr merge, clang-format, bzr commit here.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 1679. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/180479785.
Appveyor build 1519. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_automate_clang_format-1519.

[SirVer (sirver)]

> Can this be added to the bunnybot merge command once we have this in trunk? I'm thinking bzr merge, clang-format, bzr commit here.

Yes, it probably can. If I can figure out how to install clang-format on the buildbot. I'll have a look.

A couple of nits inlined.

[GunChleoc (gunchleoc)]

Thanks - all fixed.

I was just thinking - we also have fix_lua_tabs.py, which is run when we update the translations.

How about having 1 central script "fix_formating.py" that does the following:

- fix_lua_tabs.py
- run_clang_format.py
- run pyformat over utils.

This script could then be called both by merge_and_push_translations.sh and bunnybot.

[SirVer (sirver)]

Sgtm.

> Am 03.12.2016 um 14:39 schrieb GunChleoc <email address hidden>:
>
> Thanks - all fixed.
>
> I was just thinking - we also have fix_lua_tabs.py, which is run when we update the translations.
>
> How about having 1 central script "fix_formating.py" that does the following:
>
> - fix_lua_tabs.py
> - run_clang_format.py
> - run pyformat over utils.
>
> This script could then be called both by merge_and_push_translations.sh and bunnybot.
> --
> https://code.launchpad.net/~widelands-dev/widelands/automate_clang-format/+merge/312287
> You are requested to review the proposed merge of lp:~widelands-dev/widelands/automate_clang-format into lp:widelands.

[GunChleoc (gunchleoc)]

Will do that in this branch then.

[GunChleoc (gunchleoc)]

Done. Never mind the big diff for udate_authors.py, the relevant bit is the removed call at the bottom. Pyformat already in action here ;)

review: Needs Resubmitting

[SirVer (sirver)]

pretty hard to review this diff with the formatting changes. I found two nits, I think otherwise this is ready to merge.

review: Approve

[GunChleoc (gunchleoc)]

Thanks for the review!

So, pyformat doesn't fix long lines, need to keep that in mind.

@bunnybot merge

=== renamed file 'utils/fix_lua_tabs.py' => 'utils/fix_formatting.py'
--- utils/fix_lua_tabs.py	2015-10-31 12:11:44 +0000
+++ utils/fix_formatting.py	2016-12-04 06:44:59 +0000
@@ -2,29 +2,38 @@
 # -*- coding: utf-8 -*-
 
 
-"""The code base had inconsistent usage of tabs/spaces for indenting in Lua
+"""The code base had inconsistent usage of tabs/spaces for indenting in Lua.
+
 files. Spaces were more prominent - and I prefer them over tabs. So I wrote
 this small script to fix leading tabs in Lua files to spaces.
 
 It also saves files in unix file endings ("\r\n") and strips empty lines at the
 end of files and whitespace characters at the end of lines.
+
+After fixing the Lua tabs, this script also executes clang-format over the src
+directory and pyformat over the utils directory.
+
 """
 
 import argparse
 import os
 import re
 import sys
+from subprocess import call
 
 LEADING_TABS = re.compile(r'^\s*\t+\s*')
 PYTHON3 = sys.version_info >= (3, 0)
 SPACES_PER_TAB = 3
 
+
 def parse_args():
-    p = argparse.ArgumentParser(description=
-        "Fix common whitespace errors in Lua files. Recurses over all Lua files."
-    )
+    p = argparse.ArgumentParser(
+        description='Fix common whitespace errors in Lua files, run clang-format'
+        ' over the code base and pyformat over the utils directory.'
+        ' Recurses over all relevant files.')
     return p.parse_args()
 
+
 def read_text_file(filename):
     """Reads the contens of a text file."""
     if PYTHON3:
@@ -32,6 +41,7 @@
     else:
         return open(filename, 'r').read().decode('utf-8')
 
+
 def write_text_file(filename, content):
     """Writes 'content' into a text file."""
     if PYTHON3:
@@ -39,30 +49,52 @@
     else:
         open(filename, 'w').write(content.encode('utf-8'))
 
-def find_lua_files():
-    for (dirpath, _, filenames) in os.walk("."):
+
+def find_files(startpath, extensions):
+    for (dirpath, _, filenames) in os.walk(startpath):
         for filename in filenames:
-            if os.path.splitext(filename)[-1].lower() == ".lua":
+            if os.path.splitext(filename)[-1].lower() in extensions:
                 yield os.path.join(dirpath, filename)
 
+
 def main():
     parse_args()
 
-    if not os.path.isdir("src") or not os.path.isdir("utils"):
-        print("CWD is not the root of the repository.")
+    if not os.path.isdir('src') or not os.path.isdir('utils'):
+        print('CWD is not the root of the repository.')
         return 1
 
-    for filename in find_lua_files():
-        print "Fixing %r" % filename
-        lines = read_text_file(filename).strip().split("\n")
+    sys.stdout.write('Fixing Lua tabs ')
+    for filename in find_files('.', ['.lua']):
+        sys.stdout.write('.')
+        sys.stdout.flush()
+        lines = read_text_file(filename).strip().split('\n')
         new_lines = []
         for line in lines:
             m = LEADING_TABS.match(line)
             if m is not None:
-                line = line[m.start():m.end()].expandtabs(SPACES_PER_TAB) + line[m.end():]
-            new_lines.append(line.rstrip() + "\n")
-        write_text_file(filename, "".join(new_lines))
+                line = line[m.start():m.end()].expandtabs(
+                    SPACES_PER_TAB) + line[m.end():]
+            new_lines.append(line.rstrip() + '\n')
+        write_text_file(filename, ''.join(new_lines))
+    print(' done.')
+
+    sys.stdout.write('\nFormatting C++ ')
+    for filename in find_files('./src', ['.cc', '.h']):
+        sys.stdout.write('.')
+        sys.stdout.flush()
+        call(['clang-format', '-i', filename])
+    print(' done.')
+
+    sys.stdout.write('\nFormatting Python utils ')
+    for filename in find_files('./utils', ['.py']):
+        sys.stdout.write('.')
+        sys.stdout.flush()
+        call(['pyformat', '-i', filename])
+    print(' done.')
+
+    print 'Formatting finished.'
     return 0
 
-if __name__ == "__main__":
+if __name__ == '__main__':
     sys.exit(main())

=== modified file 'utils/merge_and_push_translations.sh'
--- utils/merge_and_push_translations.sh	2016-07-25 11:47:16 +0000
+++ utils/merge_and_push_translations.sh	2016-12-04 06:44:59 +0000
@@ -57,6 +57,16 @@
   exit 1;
 fi
 
+# Fix formatting
+utils/fix_formatting.py
+if [ $? -eq 0 ]
+then
+  echo "Fixed formatting";
+else
+  echo "Failed to fix formatting";
+  exit 1;
+fi
+
 # Fix line breaks.
 # TODO(GunChleoc): We hope that Transifex will fix these already.
 # This script can be removed if we don't get any errors in the future.

=== modified file 'utils/update_authors.py'
--- utils/update_authors.py	2016-11-12 17:59:50 +0000
+++ utils/update_authors.py	2016-12-04 06:44:59 +0000
@@ -13,25 +13,26 @@
 # and writes the translator and developer credits to ./txts/developers.lua
 # The locale information is written to ../data/i18n/locales.lua.
 
-base_path = os.path.abspath(os.path.join(os.path.dirname(__file__),os.path.pardir))
-
-print("Reading locales from JSON:")
-
-source_path = os.path.normpath(base_path + "/data/i18n/locales")
+base_path = os.path.abspath(os.path.join(
+    os.path.dirname(__file__), os.path.pardir))
+
+print('Reading locales from JSON:')
+
+source_path = os.path.normpath(base_path + '/data/i18n/locales')
 
 if (not os.path.isdir(source_path)):
-	print("Error: Path " + source_path + " not found.")
-	sys.exit(1)
+    print('Error: Path ' + source_path + ' not found.')
+    sys.exit(1)
 
 # Each language's translators live in a separate file, so we list the dir
 source_files = sorted(os.listdir(source_path), key=str.lower)
 
-lua_locales = "-- This file is generated by utils/update_authors.py.\n"
-lua_locales += "-- The locale data is managed in Transifex.\n\n"
-lua_locales += "return {\n"
-lua_locales += "\t-- Locales are identified by their ISO code.\n"
+lua_locales = '-- This file is generated by utils/update_authors.py.\n'
+lua_locales += '-- The locale data is managed in Transifex.\n\n'
+lua_locales += 'return {\n'
+lua_locales += '\t-- Locales are identified by their ISO code.\n'
 lua_locales += ' \ten = {\n'
-lua_locales += "\t\t-- Used to display the locale in the Options menu.\n"
+lua_locales += '\t\t-- Used to display the locale in the Options menu.\n'
 lua_locales += '\t\tname = "English",\n\n'
 lua_locales += "\t\t-- Defines the language's position on the list in the Options menu.\n"
 lua_locales += '\t\tsort_name = "English",\n\n'
@@ -39,117 +40,122 @@
 lua_locales += '\t\tfont = "default"\n'
 lua_locales += '\t},\n'
 
-lua_translators = ""
-lua_translators += "function translators() return {" # developers
+lua_translators = ''
+lua_translators += 'function translators() return {'  # developers
 
 for source_filename in source_files:
-	# Only json files, and not the template file please
-	if source_filename.endswith(".json") and source_filename != "locales_translators.json":
-		source_file = open(source_path + "/" + source_filename, "r")
-		translators = json.load(source_file)
-		locale_message = "- Added"
-
-		# Parsing translator credits
-		# Make sure we don't pick up untranslated stuff
-		if translators["translator-list"] != 'translator-credits':
-			locale_message += " translators and"
-			lua_translators += '{' # entry
-			lua_translators += 'heading = "' + translators["your-language-name"]
-			if translators["your-language-name-in-english"] != 'English' and translators["your-language-name-in-english"] != translators["your-language-name"] :
-				lua_translators += ' (' + translators["your-language-name-in-english"] + ')'
-			lua_translators += '",'
-			lua_translators += 'entries = {' # entries
-			lua_translators += '{' # entry
-			lua_translators += 'members = {' # members
-			for transl_name in translators["translator-list"].split("\n"):
-				lua_translators += '"' + transl_name + '",'
-			lua_translators += "}," # members
-			lua_translators += "}," # entry
-			lua_translators += "}," # entries
-			lua_translators += "}," # entry
-
-		# Parsing locale info
-		# Make sure we don't pick up untranslated stuff
-		locale_code = source_filename.split(".json")[0]
-		locale_message += " locale info for " + locale_code
-		lua_locales += '\n\t' + locale_code + ' = {\n' # entry with locale code
-
-		if translators["your-language-name"] != 'English' or locale_code == 'en':
-			lua_locales += '\t\tname = "' + translators["your-language-name"] + '",\n'
-		else:
-			lua_locales += '\t\tname = "' + locale_code + '",\n'
-
-		if translators["language-sort-name"] != 'English' or locale_code == 'en':
-			lua_locales += '\t\tsort_name = "' + translators["language-sort-name"] + '",\n'
-		else:
-			lua_locales += '\t\tsort_name = "' + locale_code + '",\n'
-
-		lua_locales += '\t\tfont = "' + translators["font-set"] + '"\n'
-		lua_locales += "\t},\n" # entry
-		print(locale_message)
-lua_locales += "}\n"
-
-lua_translators += "} end" # developers
-
-print("Writing locales\n")
-dest_filename = "locales.lua"
-dest_filepath = os.path.normpath(base_path + "/data/i18n") + "/" + dest_filename
+    # Only json files, and not the template file please
+    if source_filename.endswith('.json') and source_filename != 'locales_translators.json':
+        source_file = open(source_path + '/' + source_filename, 'r')
+        translators = json.load(source_file)
+        locale_message = '- Added'
+
+        # Parsing translator credits
+        # Make sure we don't pick up untranslated stuff
+        if translators['translator-list'] != 'translator-credits':
+            locale_message += ' translators and'
+            lua_translators += '{'  # entry
+            lua_translators += 'heading = "' + \
+                translators['your-language-name']
+            if translators['your-language-name-in-english'] != 'English' and translators['your-language-name-in-english'] != translators['your-language-name']:
+                lua_translators += ' (' + \
+                    translators['your-language-name-in-english'] + ')'
+            lua_translators += '",'
+            lua_translators += 'entries = {'  # entries
+            lua_translators += '{'  # entry
+            lua_translators += 'members = {'  # members
+            for transl_name in translators['translator-list'].split('\n'):
+                lua_translators += '"' + transl_name + '",'
+            lua_translators += '},'  # members
+            lua_translators += '},'  # entry
+            lua_translators += '},'  # entries
+            lua_translators += '},'  # entry
+
+        # Parsing locale info
+        # Make sure we don't pick up untranslated stuff
+        locale_code = source_filename.split('.json')[0]
+        locale_message += ' locale info for ' + locale_code
+        lua_locales += '\n\t' + locale_code + \
+            ' = {\n'  # entry with locale code
+
+        if translators['your-language-name'] != 'English' or locale_code == 'en':
+            lua_locales += '\t\tname = "' + \
+                translators['your-language-name'] + '",\n'
+        else:
+            lua_locales += '\t\tname = "' + locale_code + '",\n'
+
+        if translators['language-sort-name'] != 'English' or locale_code == 'en':
+            lua_locales += '\t\tsort_name = "' + \
+                translators['language-sort-name'] + '",\n'
+        else:
+            lua_locales += '\t\tsort_name = "' + locale_code + '",\n'
+
+        lua_locales += '\t\tfont = "' + translators['font-set'] + '"\n'
+        lua_locales += '\t},\n'  # entry
+        print(locale_message)
+lua_locales += '}\n'
+
+lua_translators += '} end'  # developers
+
+print('Writing locales\n')
+dest_filename = 'locales.lua'
+dest_filepath = os.path.normpath(
+    base_path + '/data/i18n') + '/' + dest_filename
 dest_file = codecs.open(dest_filepath, encoding='utf-8', mode='w')
 dest_file.write(lua_locales)
 
-print("Writing translators\n")
-dest_filename = "translators_data.lua"
-dest_filepath = os.path.normpath(base_path + "/data/txts") + "/" + dest_filename
+print('Writing translators\n')
+dest_filename = 'translators_data.lua'
+dest_filepath = os.path.normpath(
+    base_path + '/data/txts') + '/' + dest_filename
 dest_file = codecs.open(dest_filepath, encoding='utf-8', mode='w')
 dest_file.write(lua_translators)
 
-print("Reading developers from JSON")
-source_path = os.path.normpath(base_path + "/data/txts")
+print('Reading developers from JSON')
+source_path = os.path.normpath(base_path + '/data/txts')
 
 if (not os.path.isdir(source_path)):
-	print("Error: Path " + source_path + " not found.")
-	sys.exit(1)
+    print('Error: Path ' + source_path + ' not found.')
+    sys.exit(1)
 
-source_file = open(source_path + "/developers.json", "r")
-developers = json.load(source_file)["developers"]
+source_file = open(source_path + '/developers.json', 'r')
+developers = json.load(source_file)['developers']
 
 lua_string = """-- Do not edit this file - it is automatically generated
 -- by utils/update_authors.py from developers.json.
 """
-lua_string += "function developers() return {" # developers
+lua_string += 'function developers() return {'  # developers
 
 for category in developers:
-	print("- Adding " + category["heading"])
-	if category["heading"] != "Translators": # Unused hook for adding translators
-		lua_string += '{' # category
-		lua_string += 'heading = _"' + category["heading"] + '",' # This will be localized
-		lua_string += 'image = "' + category["image"] + '",'
-
-		lua_string += 'entries = {' # entries
-
-		for subcategory in category["entries"]:
-			lua_string += '{' # entry
-			if 'subheading' in subcategory:
-				lua_string += 'subheading = _"' + subcategory["subheading"] + '",' # This will be localized
-
-			lua_string += 'members = {' # members
-			for member in subcategory["members"]:
-				lua_string += '"' + member + '",'
-			lua_string += "}," # members
-
-			lua_string += "}," # entry
-		lua_string += "}," # entries
-
-		lua_string += "}," # category
-lua_string += "} end\n" # developers
-
-print("Writing developers")
-dest_filename = "developers.lua"
-dest_filepath = source_path + "/" + dest_filename
+    print('- Adding ' + category['heading'])
+    if category['heading'] != 'Translators':  # Unused hook for adding translators
+        lua_string += '{'  # category
+        lua_string += 'heading = _"' + \
+            category['heading'] + '",'  # This will be localized
+        lua_string += 'image = "' + category['image'] + '",'
+
+        lua_string += 'entries = {'  # entries
+
+        for subcategory in category['entries']:
+            lua_string += '{'  # entry
+            if 'subheading' in subcategory:
+                lua_string += 'subheading = _"' + \
+                    subcategory['subheading'] + '",'  # This will be localized
+
+            lua_string += 'members = {'  # members
+            for member in subcategory['members']:
+                lua_string += '"' + member + '",'
+            lua_string += '},'  # members
+
+            lua_string += '},'  # entry
+        lua_string += '},'  # entries
+
+        lua_string += '},'  # category
+lua_string += '} end\n'  # developers
+
+print('Writing developers')
+dest_filename = 'developers.lua'
+dest_filepath = source_path + '/' + dest_filename
 dest_file = codecs.open(dest_filepath, encoding='utf-8', mode='w')
 dest_file.write(lua_string)
-
-print("Fixing the formatting")
-import fix_lua_tabs
-fix_lua_tabs.main()
-print("Done.")
+print('Done.')