[webkit-reviews] review granted: [Bug 34379] check-webkit-style: Break the style error handlers out into a separate class : [Attachment 47802] Proposed patch

[bugzilla-daemon at webkit.org]

Shinichiro Hamaji <hamaji at chromium.org> has granted Chris Jerdonek
<cjerdonek at webkit.org>'s request for review:
Bug 34379: check-webkit-style: Break the style error handlers out into a
separate class
https://bugs.webkit.org/show_bug.cgi?id=34379

Attachment 47802: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=47802&action=review

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
> -	       handle_style_error =
self._default_style_error_handler(file_path)
> -
> +	       handle_style_error = DefaultStyleErrorHandler(file_path,
> +					self.options,
> +					self._increment_error_count,
> +					self._stderr_write)

I'm not sure if we usually break lines in this way.

handle_style_error = DefaultStyleErrorHandler(file_path,
					      self.options,
					      self._increment_error_count,
					      self._stderr_write)

or

handle_style_error = DefaultStyleErrorHandler(file_path, self.options,
self._increment_error_count, self._stderr_write)

?

> +	   if line_number not in self._get_line_numbers():
> +	       # Then the error is not reportable.
> +	       return
> +
> +	   self._default_error_handler(line_number, category, confidence,
> +				       message)
> +

Did we fix this FIXME?

# FIXME: Make sure errors having line number zero are
#	 logged -- like carriage-return errors.