Bug 34189

Summary:

check-webkit-style failed to complain about missing braces

Product:

WebKit

Reporter:

Evan Martin <evan>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, aivopaas, bburg, cjerdonek, commit-queue, eric, glenn, hamaji, levin, llango.u-szeged, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	none
patch for landing	none

Evan Martin

Reported 2010-01-26 16:07:00 PST

This patch: https://bugs.webkit.org/show_bug.cgi?id=34186#c1 had this diff: ASSERT(U_SUCCESS(error)); + for (unsigned i = 0; i < normalizedString.length(); ++i) + if (Font::treatAsSpace(m_normalizedBuffer[i])) + m_normalizedBuffer[i] = ' '; Which I'm informed lacks parens. But the style bot didn't mind it.

Attachments
Patch (5.47 KB, patch) 2014-03-06 05:42 PST, László Langó	no flags	Details Formatted Diff Diff
patch for landing (5.52 KB, patch) 2014-03-11 07:12 PDT, László Langó	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Evan Martin

Comment 1 2010-01-26 16:07:13 PST

Er, make that: "... which lacks *braces*."

Aivo Paas

Comment 2 2012-11-11 23:56:12 PST

@@ -1862,7 +1862,8 @@ bool EventHandler::handleMouseMoveEvent(const PlatformMouseEvent& mouseEvent, Hi if (FrameView* view = m_frame->view()) { OptionalCursor optionalCursor = selectCursor(mev, scrollbar); if (optionalCursor.isCursorChange()) - view->setCursor(optionalCursor.cursor()); + m_currentMouseCursor = optionalCursor.cursor(); + view->setCursor(m_currentMouseCursor); } }

Blaze Burg

Comment 3 2014-03-02 11:20:29 PST

Ran into this again in review of https://bugs.webkit.org/show_bug.cgi?id=129388

László Langó

Comment 4 2014-03-06 01:51:50 PST

I have a draft patch for this. It works fine on the examples you mentioned previously, but it complains about this: if (missedCues[i].data()->startTime() < missedCues[i].data()->endTime()) eventTasks.append(std::make_pair(missedCues[i].data()->endTime(), missedCues[i].data())); I'm a bit confused. Is this false positive or not? Should we use braces in this case?

László Langó

Comment 5 2014-03-06 05:42:00 PST

Created attachment 225982 [details] Patch

László Langó

Comment 6 2014-03-06 05:43:00 PST

(In reply to comment #4) > I have a draft patch for this. It works fine on the examples you mentioned previously, but it complains about this: > > if (missedCues[i].data()->startTime() < missedCues[i].data()->endTime()) > eventTasks.append(std::make_pair(missedCues[i].data()->endTime(), > missedCues[i].data())); > > I'm a bit confused. Is this false positive or not? Should we use braces in this case? Meanwhile I found the answer here: http://www.webkit.org/coding/coding-style.html#braces-one-line

Ryosuke Niwa

Comment 7 2014-03-11 00:42:55 PDT

Comment on attachment 225982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225982&action=review rs=me. > Tools/Scripts/webkitpy/style/checkers/cpp.py:2407 > + if (match(r'^\s*\b(if|for|foreach|while|else)\b\s', line) We don't want foreach here.

László Langó

Comment 8 2014-03-11 07:12:07 PDT

Created attachment 226423 [details] patch for landing

WebKit Commit Bot

Comment 9 2014-03-11 07:50:55 PDT

Comment on attachment 226423 [details] patch for landing Clearing flags on attachment: 226423 Committed r165451: <http://trac.webkit.org/changeset/165451>

WebKit Commit Bot

Comment 10 2014-03-11 07:50:59 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.