New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 690157 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS:
Pri: 2
Type: Bug



Sign in to add a comment

clang-format quality problem: ObjC prefers to break between receiver and selector, rather than arguments

Project Member Reported by rsesek@chromium.org, Feb 8 2017

Issue description

clang-format produced code that (choose all that apply): 
[x] Doesn't match Chromium style
- Doesn't match blink style
[x] Riles my finely honed stylistic dander
[x] No sane human would ever choose

Here's the code before formatting:

  [destWindow setCollectionBehavior:
      NSWindowCollectionBehaviorMoveToActiveSpace];

  item->menu_item.reset(
      [[NSMenuItem alloc] initWithTitle:base::SysUTF16ToNSString(title)
                                 action:nil
                          keyEquivalent:@""]);

Here's the code after formatting:

   [destWindow
       setCollectionBehavior:NSWindowCollectionBehaviorMoveToActiveSpace];

  item->menu_item.reset([[NSMenuItem alloc]
      initWithTitle:base::SysUTF16ToNSString(title)
             action:nil
      keyEquivalent:@""]);


Here's how it ought to look:

  [destWindow setCollectionBehavior:
      NSWindowCollectionBehaviorMoveToActiveSpace];

  item->menu_item.reset(
      [[NSMenuItem alloc] initWithTitle:base::SysUTF16ToNSString(title)
                                 action:nil
                          keyEquivalent:@""]);

Code review link for full files/context:
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/browser_window_controller_private.mm?q=browser_window_controller_private.mm&sq=package:chromium&dr&l=432

https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/history_menu_bridge.mm?sq=package:chromium&dr&l=295

Breaking after the receiver looks wrong, as in Chromium we prefer to break after the argument.
 
Here's another sample that is made less readable:

-  [minimizeButton_ setFrameOrigin:NSMakePoint(
-      totalWidth - kPaddingHorizontal - NSWidth([minimizeButton_ frame]),
-      (totalHeight - NSHeight([minimizeButton_ frame])) / 2)];
+  [minimizeButton_
+      setFrameOrigin:NSMakePoint(
+                         totalWidth - kPaddingHorizontal -
+                             NSWidth([minimizeButton_ frame]),
+                         (totalHeight - NSHeight([minimizeButton_ frame])) /
+                             2)];

Filed upstream as https://llvm.org/bugs/show_bug.cgi?id=31906.
Components: Tools
Labels: clang-format
Labels: Pri-2
Issue has a component, but no priority. Updating to have default priority (Pri-2)

Sign in to add a comment