Summary: | WebCore/page/ContextMenuController.cpp does not conform to style standards | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Fenton <mifenton> | ||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | levin, manyoso | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Mike Fenton
2009-07-23 12:03:09 PDT
Created attachment 33350 [details]
Style patch for ContextMenuController.cpp
Comment on attachment 33350 [details]
Style patch for ContextMenuController.cpp
This would be much easier to read if we had a white-space ignoring diff. It's difficult for me to tell that you haven' actually changed any code here. :(
The main change in the patch is to fix the indentation of the case statements. It also fixes the include order and a few whitespace at end of line fixes. Comment on attachment 33350 [details] Style patch for ContextMenuController.cpp > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > +2009-07-23 Mike Fenton <mike.fenton@torchmobile.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Update WebCore/page/ContextMenuController.cpp to conform to WebKit > + Style Guidelines as identified by cpplint.py. Now it is cpp_style.py :) > diff --git a/WebCore/page/ContextMenuController.cpp b/WebCore/page/ContextMenuController.cpp Since this is about fixing styling in ContextMenuController.cpp, Why not fix the indentation in this block of code? Personally, I would unwrap this line but you could just indent to the createWindow(. 107 static void openNewWindow(const KURL& urlToLoad, Frame* frame) 108 { 109 if (Page* oldPage = frame->page()) { 110 WindowFeatures features; 111 if (Page* newPage = oldPage->chrome()->createWindow(frame, 112 FrameLoadRequest(ResourceRequest(urlToLoad, frame->loader()->outgoingReferrer())), features)) 113 newPage->chrome()->show(); 114 } 115 } > + case ContextMenuItemTagOpenFrameInNewWindow: { > + DocumentLoader* loader = frame->loader()->documentLoader(); > + if (!loader->unreachableURL().isEmpty()) > + openNewWindow(loader->unreachableURL(), frame); > + else > + openNewWindow(loader->url(), frame); > + break; > } The indentation for the "}" needs fixing. > + case ContextMenuItemTagSpellingGuess: > + ASSERT(frame->selectedText().length()); > + if (frame->editor()->shouldInsertText(item->title(), frame->selection()->toNormalizedRange().get(), > + EditorInsertActionPasted)) { Either indent to the level of the nested paren or unwrap. > + Document* document = frame->document(); > + RefPtr<ReplaceSelectionCommand> command = > + ReplaceSelectionCommand::create(document, createFragmentFromMarkup(document, item->title(), ""), > + true, false, true); Either indent to the level of the nested paren or unwrap. > + case ContextMenuItemTagOpenLink: > + if (Frame* targetFrame = result.targetFrame()) { > + targetFrame->loader()->loadFrameRequest(FrameLoadRequest(ResourceRequest(result.absoluteLinkURL(), > + frame->loader()->outgoingReferrer())), false, false, 0, 0); Either indent to the level of the nested paren or unwrap. Created attachment 33714 [details]
Replacement patch for ContextMenuController.cpp style fixes.
Thanks for the review.
This updated patch has all the noted changes corrected. Functions have been unwrapped and the indentation of the brace corrected.
Committed as http://trac.webkit.org/changeset/46544 |