Make run-webkit-lint supports files as arguments so that we dont need to call python WebKitTools/Scripts/modules/cpplint.py on individual files. Patch coming up.
Created attachment 33274 [details] Make run-webkit-lint supports files as arguments
Comment on attachment 33274 [details] Make run-webkit-lint supports files as arguments Thanks for the patch. This is a nice addition. Just a few things to address at the moment. Well, first off, sorry, I changed the name of the tool on you. So if you would switch the patch over to check-webkit-style, that would be excellent. fwiw, you don't need to request me as the reviewer - Adam has been also reviewing these and doing a great job (The funny thing is that it delayed your review because I typically scan the general queue -- not used to being asked for by name). cc'ing me did get my attention though :) > diff --git a/WebKitTools/Scripts/run-webkit-lint b/WebKitTools/Scripts/run-webkit-lint Please fix up "cpplint._USAGE" to reflect the fact that you may optionally pass in files. What should happen if the user passes in --git-commit and files? (Personally I recommend something simple like an error message that you need to choose one or the other.) > + if not filenames: > + cwd = os.path.abspath('.') > + scm = detect_scm_system(cwd) > + > + process_patch(scm.create_patch()) > + else: > + # Change stderr to write with replacement characters so we don't die > + # if we try to print something containing non-ASCII characters. > + sys.stderr = codecs.StreamReaderWriter(sys.stderr, > + codecs.getreader('utf8'), > + codecs.getwriter('utf8'), > + 'replace') If this is useful (and I think it is), it seems like it would be needed for both code paths, so I'd move this outside of the "if". > + > + for filename in filenames: > + cpplint.process_file(filename)
Created attachment 33712 [details] Updates and improved patch
Comment on attachment 33712 [details] Updates and improved patch I'm willing to do these changes, but I'd like to know how you feel about the "consider" comments. > diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style > + The file parameter is optional and multiple file scan be passed in. Leaving Consider: The file parameter is optional and multiple files to scan may be passed in > + out the file parameter will apply the check to the files currently not Consider: out the file parameter will apply the check to the files changed according to the scm system. > + if files and "--git-commit" in flags: > + sys.stderr.write("ERROR: It is not possible to check files " > + "and a specific commit at the same time.\n" + cpp_style._USAGE) > sys.exit(1) > + elif files: Make this an "if" since the pre if "exits".
I feel fine about these changes.
Comment on attachment 33712 [details] Updates and improved patch Landed (with the above changes) in 46542.