[Webkit-unassigned] [Bug 32538] [check-webkit-style] Add support for TAB check against text files

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 14 21:23:26 PST 2009


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





--- Comment #4 from Shinichiro Hamaji <hamaji at chromium.org>  2009-12-14 21:23:25 PST ---
(From update of attachment 44837)
Though I'm not a reviewer, I'd like to put some comments. First of all, thanks
for doing this. Actually, I wanted this feature long time...

As Eric mentioned, we may want to consider allow-tabs property. Maybe we can
just use svn proplist for svn repositories, but are there any good ways to see
the properties from git repository? If we cannot find a good solution, we may
be able start from checking just ChangeLog, where I think most tab mistakes are
done.

PEP8 says that we should put 2 blank lines between top level definitions.
Though I don't know WebKit is using this style (I bet there're no official rule
for python), I think it would be good to keep text_style.py consistent with
cpp_style.py.

http://www.python.org/dev/peps/pep-0008/

> +        elif "ChangeLog" in filename or file_extension in ['.html', '.js', '.py', '.txt']:

We may want to add '.pm', '.php', and 'WebKitTools/Scripts/*' ?

> +            line_numbers = set();
> +            for line in diff.lines:
> +                # When deleted line is not set, it means that
> +                # the line is newly added.
> +                if not line[0]:
> +                    line_numbers.add(line[1])
> +
> +            def error_for_patch(filename, line_number, message):
> +                if line_number in line_numbers:
> +                    text_style.error(filename, line_number, message)
> +
> +            text_style.process_file(filename, error=error_for_patch)

We can factor out this code and the similar code for cpp_style? As for the
place of initialization of line_numbers, David suggested me that we should
delay the initialization so that we don't need to initialize line_numbers if
there are no errors. By the way, I think it would be better to set None instead
of set() as the initial value of line_numbers.

> +def error_count():
> +    """Returns the global caount of erported errors."""

typo: "global count of reported errors"?

> -- 
> 1.6.3.3

I'm not sure what's this :)

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



More information about the webkit-unassigned mailing list