[Webkit-unassigned] [Bug 27291] Support lint for patches

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27291


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33158|review?                     |review-
               Flag|                            |




--- Comment #9 from David Levin <levin at chromium.org>  2009-07-21 02:39:10 PDT ---
(From update of attachment 33158)
A few things to consider....

> diff --git a/WebKitTools/Scripts/modules/cpplint.py b/WebKitTools/Scripts/modules/cpplint.py

> +# FIXME: Once run-webkit-lint script becomes ready, we don't need to
> +#        support standalone execution for this module.
> +#        So, we should remove _USAGE and main() eventually.

I'm not sure this is necessary (maybe).


>      _cpplint_state.reset_error_count()

Add this line here:

   _set_verbose_level(_cpplint_state.verbose_level)

>      for filename in filenames:
> -        process_file(filename, _cpplint_state.verbose_level)
> +        process_file(filename)


> diff --git a/WebKitTools/Scripts/run-webkit-lint b/WebKitTools/Scripts/run-webkit-lint

> +cpplint._USAGE = """
> +Syntax: webkit-run-lint [--verbose=#] [--output=vs7] [--filter=-x,+y,...]
> +
> +  The style guidelines this tries to follow are those in
> +    http://webkit.org/coding/coding-style.html
> +
> +  Every problem is given a confidence score from 1-5, with 5 meaning we are
> +  certain of the problem, and 1 meaning it could be a legitimate construct.
> +  This will miss some errors, and is not a substitute for a code review.
> +
> +  To prevent specific lines from being linted, add a '// NOLINT' comment to the
> +  end of the line.
> +
> +  Linted extensions are .cc, .cpp, and .h.  Other file types will be ignored.
> +
> +  Flags:
> +
> +    output=vs7
> +      By default, the output is formatted to ease emacs parsing.  Visual Studio
> +      compatible output (vs7) may also be used.  Other formats are unsupported.
> +
> +    verbose=#
> +      Specify a number 0-5 to restrict errors to certain verbosity levels.
> +
> +    filter=-x,+y,...
> +      Specify a comma-separated list of category-filters to apply: only
> +      error messages whose category names pass the filters will be printed.
> +      (Category names are printed with the message and look like
> +      "[whitespace/indent]".)  Filters are evaluated left to right.
> +      "-FOO" and "FOO" means "do not print categories that start with FOO".
> +      "+FOO" means "do print categories that start with FOO".
> +
> +      Examples: --filter=-whitespace,+whitespace/braces
> +                --filter=whitespace,runtime/printf,+runtime/printf_format
> +                --filter=-,+build/include_what_you_use
> +
> +      To see a list of all the categories used in cpplint, pass no arg:
> +         --filter=
> +"""

When I said this:  Consider doing this

""" ....""" % {'program_name' sys.argv[0]}
In the description text replace "webkit-run-lint" and "cpplint" with
"%{program_name}s"

I was referring to this comment (not the one in cpplint.py).  It would fix two
things: "webkit-run-lint" is not the program name. "cpplint" is also not the
name of the program being run.
sys.argv[0] is the program name.

>
> +        if file_extension in ['.cc', '.cpp', '.h']:
> +            line_numbers = set()
This looks fine.  The problem with my suggestion was the assigment inside of
error_for_patch which made the variable have scope local to that block.


> +    if args:
> +        sys.stderr.write('ERROR: We don\'t support files as arguments for now.\n' + cpplint._USAGE)

Consider using "" for the outside quotes and change \' to '.  Like this

        sys.stderr.write("ERROR: We don't support files as arguments for
now.\n" + cpplint._USAGE)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.