RESOLVED FIXED 34189
check-webkit-style failed to complain about missing braces 
https://bugs.webkit.org/show_bug.cgi?id=34189
Summary check-webkit-style failed to complain about missing braces
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
DetailsFormatted DiffDiff
patch for landing (5.52 KB, patch)
2014-03-11 07:12 PDT, László Langó 		no flags
DetailsFormatted DiffDiff
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.