Summary: | [WinCairo] drawLineForMisspellingOrBadGrammar is Not Functional | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | mrobinson, webkit.review.bot | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | Windows XP | ||||||||||||||||
Attachments: |
|
Description
Brent Fulgham
2010-06-28 18:10:28 PDT
Created attachment 60030 [details]
Patch enabling 'squiggle' for bad spelling/grammar.
Patch implements drawing logic for creating a squiggly line beneath misspelled or ungrammatical content during editing. Logic is based on the Pango sources, so that the WinCairo port matches behavior of the other Cairo-based ports.
Great patch! The only change I would suggest is either changing the code to align with WebKit style (HEIGHT_SQUARES) or isolating it in a file that can be exempted from WebKit style checks. Created attachment 60088 [details]
Updated patch, isolating the squiggle code in its own LGPL-licensed file.
Update based on everyone's comments up to this point.
Attachment 60088 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:24: Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:70: Extra space before ( in function call [whitespace/parens] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:74: One line control clauses should not use braces. [whitespace/braces] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:97: One space before end of line comments [whitespace/comments] [5]
Total errors found: 4 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 60161 [details]
Silence some style warnings.
I can't resolve the one-line if/else complaint, because the no-op (comment) line requires braces to compile. I could switch to a C-style comment to resolve, but this seems like a reasonable way to handle the stanza in question.
Attachment 60161 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:26: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:78: One line control clauses should not use braces. [whitespace/braces] [4]
Total errors found: 2 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 60260 [details]
Style cleanups.
Attachment 60260 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:24: Header file should not contain WebCore config.h. Should be: alphabetically sorted. [build/include_order] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:77: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5]
Total errors found: 2 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 60274 [details]
More style cleanups.
Revised per style check guidelines.
Attachment 60274 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:74: Tab found; better to use spaces [whitespace/tab] [1]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:75: Tab found; better to use spaces [whitespace/tab] [1]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:76: Tab found; better to use spaces [whitespace/tab] [1]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:77: Tab found; better to use spaces [whitespace/tab] [1]
Total errors found: 4 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 60278 [details]
Good Grief! Another attempt...
Comment on attachment 60278 [details]
Good Grief! Another attempt...
OK, looks sane
Landed in http://trac.webkit.org/changeset/62388. |