Bug 37881 - Numerous style violations in HTMLInputElement.cpp
Summary: Numerous style violations in HTMLInputElement.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-20 12:31 PDT by Ray
Modified: 2010-04-21 22:50 PDT (History)
1 user (show)

See Also:


Attachments
Corrections for all style test failures. (31.04 KB, patch)
2010-04-20 12:40 PDT, Ray
darin: review-
darin: commit-queue-
Details | Formatted Diff | Diff
Style changes as discussed with Darin Adler (30.33 KB, patch)
2010-04-20 15:27 PDT, Ray
no flags Details | Formatted Diff | Diff
ChangeLog update to permit build bot to check in. (30.37 KB, patch)
2010-04-21 08:34 PDT, Ray
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ray 2010-04-20 12:31:19 PDT
HTMLInputElement.cpp contains numerous style violations --- most are either misformated switch/case blocks or multi-line if statements.

I will provide a patch shortly.
Comment 1 Ray 2010-04-20 12:40:05 PDT
Created attachment 53864 [details]
Corrections for all style test failures.

This patch contains style changes only --- mostly white space and some small changes across multiple lines (for malformed if conditionals and the like).
Comment 2 Darin Adler 2010-04-20 15:02:48 PDT
Comment on attachment 53864 [details]
Corrections for all style test failures.

Since this is all about style I was picky about style.

> +         RegularExpression patternRegExp(pattern, TextCaseSensitive);
> +        int matchLength = 0;

Looks like the patternRegExp line is incorrectly indented.

> -            if (focusedInput->inputType() == RADIO && focusedInput->form() == form() &&
> -                focusedInput->name() == name())
> +            if (focusedInput->inputType() == RADIO && focusedInput->form() == form() 
> +                && focusedInput->name() == name())
>                  return false;

I think this would be better if it was just all on one long line.

> -    } else if (attr->name() == usemapAttr ||
> -               attr->name() == accesskeyAttr) {
> +    } else if (attr->name() == usemapAttr 
> +            || attr->name() == accesskeyAttr) {

I think this would be better if it was just all on one long line.

> +    } else if (attr->name() == onsearchAttr) {
> +    // Search field and slider attributes all just cause updateFromElement to be called through style recalcing.
>          setAttributeEventListener(eventNames().searchEvent, createAttributeEventListener(this, attr));

I think the comment should be indented at the same level as the code below it.

> -void HTMLInputElement::setIndeterminate(bool _indeterminate)
> +void HTMLInputElement::setIndeterminate(bool indet)

We normally use words, not abbreviations. I think "indet" is unclear. Maybe "newValue" would be better.

Otherwise looks fine. review- because you should fix at least some, preferably all, of what I mentioned above.
Comment 3 Ray 2010-04-20 15:27:20 PDT
Created attachment 53894 [details]
Style changes as discussed with Darin Adler

Only style / format changes are in this patch.
Comment 4 Ray 2010-04-20 15:28:45 PDT
(In reply to comment #2)
> (From update of attachment 53864 [details])
> Since this is all about style I was picky about style.
> [ suggestions removed for brevity]
> 
> Otherwise looks fine. review- because you should fix at least some, preferably
> all, of what I mentioned above.

Thanks, Darin! I agree with all of your comments, and have incorporated corrections for each of them.

Thanks!
Comment 5 WebKit Commit Bot 2010-04-20 15:31:00 PDT
Comment on attachment 53894 [details]
Style changes as discussed with Darin Adler

Rejecting patch 53894 from commit-queue.

Raymond.Rischpater@Nokia.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  Due to bug 30084 the commit-queue will require a restart after your change.  Please contact eseidel@chromium.org to request a commit-queue restart.  After restart the commit-queue will correctly respect your committer rights.
Comment 6 WebKit Commit Bot 2010-04-21 07:25:07 PDT
Comment on attachment 53894 [details]
Style changes as discussed with Darin Adler

Rejecting patch 53894 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '53894', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Logging in as eseidel@chromium.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=53894&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=37881&ctype=xml
Processing 1 patch from 1 bug.
Cleaning working directory
Processing patch 53894 from bug 37881.
ERROR: /Users/eseidel/Projects/CommitQueue/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Comment 7 Ray 2010-04-21 08:34:39 PDT
Created attachment 53960 [details]
ChangeLog update to permit build bot to check in.

ChangeLog now contains reviewer name to permit the BuildBot to take the changes. 
No changes to the code submission.
Comment 8 Darin Adler 2010-04-21 09:20:21 PDT
Comment on attachment 53960 [details]
ChangeLog update to permit build bot to check in.

It's not actually correct to put the reviewer's name in the change log. Instead you are supposed to just leave the Reviewed by NOBODY line in.
Comment 9 Eric Seidel (no email) 2010-04-21 18:11:14 PDT
Comment on attachment 53894 [details]
Style changes as discussed with Darin Adler

Cleared Darin Adler's review+ from obsolete attachment 53894 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 10 WebKit Commit Bot 2010-04-21 22:50:07 PDT
Comment on attachment 53960 [details]
ChangeLog update to permit build bot to check in.

Clearing flags on attachment: 53960

Committed r58057: <http://trac.webkit.org/changeset/58057>
Comment 11 WebKit Commit Bot 2010-04-21 22:50:12 PDT
All reviewed patches have been landed.  Closing bug.