[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak marked 6 inline comments as done.
jolesiak added inline comments.


================
Comment at: lib/Format/FormatToken.h:247-248
+  /// If this is the first ObjC selector name in an ObjC method
+  /// definition or call, this contains the number of parts that whole selector
+  /// consist of.
   unsigned ObjCSelectorNameParts = 0;
----------------
benhamilton wrote:
> Grammar nit: that whole selector consist of -> that the whole selector consists of
> 
Thanks!


================
Comment at: lib/Format/TokenAnnotator.cpp:519
+        // FirstObjCSelectorName is set when a colon is found. This does
+        // not work, however, when method has no parameters.
+        // Here, we set FirstObjCSelectorName when the end of the expression is
----------------
benhamilton wrote:
> Grammar nit-pick: when method -> when the method
> 
Thanks!


================
Comment at: lib/Format/TokenAnnotator.cpp:520
+        // not work, however, when method has no parameters.
+        // Here, we set FirstObjCSelectorName when the end of the expression is
+        // reached, in case it was not set already.
----------------
benhamilton wrote:
> expression -> method call
> 
Thanks!


================
Comment at: lib/Format/TokenAnnotator.cpp:627
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+    // For ObjC methods number of parameters is calculated differently as
+    // method declarations have different structure (parameters are not inside
----------------
benhamilton wrote:
> Grammar nit-pick: methods number -> methods, the number
> 
Thanks!


================
Comment at: lib/Format/TokenAnnotator.cpp:628
+    // For ObjC methods number of parameters is calculated differently as
+    // method declarations have different structure (parameters are not inside
+    // parenthesis scope).
----------------
benhamilton wrote:
> benhamilton wrote:
> > Grammar nit-picks:
> > 
> > * have different structure -> have a different structure
> > * parameters are not -> the parameters are not
> > 
> Why does parenthesis scope matter here? `updateParameterCount()` is called from `parseSquare()`.
> 
> I'm not sure what the goal of this change is.
> 
Thanks!


================
Comment at: lib/Format/TokenAnnotator.cpp:628-629
+    // For ObjC methods number of parameters is calculated differently as
+    // method declarations have different structure (parameters are not inside
+    // parenthesis scope).
     if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)
----------------
jolesiak wrote:
> benhamilton wrote:
> > benhamilton wrote:
> > > Grammar nit-picks:
> > > 
> > > * have different structure -> have a different structure
> > > * parameters are not -> the parameters are not
> > > 
> > Why does parenthesis scope matter here? `updateParameterCount()` is called from `parseSquare()`.
> > 
> > I'm not sure what the goal of this change is.
> > 
> Thanks!
My bad, I thought the word "parenthesis" can also mean any of "(", "[", "{", "<" (not only specifically the first one).
I've replaced with "a bracket scope".

I'll comment on the goal of this change in non-inline comment.


Repository:
  rC Clang

https://reviews.llvm.org/D48716