git cl format the first third of the net/base directory

git cl format the first third of the net/base directory

This is the first step in formatting the net/ directory.

Done by running: find . | egrep '(\.c|\.h|\.m|\.mm|\.cc|\.cp|\.cpp|\.c++|\.cxx|\.hpp|\.pro|\.protodevel|\.js)$' | head -n 70 | xargs clang-format --style=chromium -i

net/base/ CLs:
* https://codereview.chromium.org/642403002/
https://codereview.chromium.org/649763002/
https://codereview.chromium.org/647883002/

BUG=422332

[jkarlin, 2014-10-10 19:02:37]

Patchset #1 (id:1) has been deleted

[jkarlin, 2014-10-10 19:33:32]

jkarlin@chromium.org changed reviewers:
+ rsleevi@chromium.org

[jkarlin, 2014-10-10 19:33:44]

Thanks Ryan!

[Ryan Sleevi, 2014-10-11 02:04:47]

Ooof. A pain to review, but thanks for looking into this.

I think we may have shaken out some clang-format suboptimalities here, and I'm
wondering if you're willing to file bugs (and/or track down the status of bugs,
as I think some may have already been filed)

See below

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
File net/base/directory_lister.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
net/base/directory_lister.h:51: enum SortType { NO_SORT, DATE, ALPHA_DIRS_FIRST,
FULL_PATH };
So, here's an example where the original code was clearer.

However, the reason why git cl format did this is because the documentation was
listed in 47-50. If we move that documentation inline, above each enum value, it
should "do the right thing"

I don't think it's worth filing a git cl format bug on it, because I think it's
the right behaviour for the general case (which is designed for inline values)

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc
File net/base/escape.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc#newc...
net/base/escape.cc:97: //   p  q  r  s  t  u  v  w  x  y  z  {  |  }  ~  <NBSP>
Seems like we should file a bug for this.

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc#newc...
net/base/escape.cc:195: : ((third_byte >= 0xA6) && (third_byte <= 0xA9)))) {
This is something that annoys rvargas@ and pkasting@, but I think is the right
thing.

Peter filed a bug on this, ISTR, as our current style guide (
http://www.chromium.org/developers/coding-style#Code_Formatting ) says wrap
_after_ operators (the existing code), not before (the new code)

I think clang-format had a config file flag added for this. Can you hunt it down
and, if so, add it to the chromium clang-format definition?

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc#newc...
net/base/escape.cc:279: 0xffffffffL}};
Oof. This formatting is awful. Might be worth a bug.

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
File net/base/filename_util_internal.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
net/base/filename_util_internal.cc:142: "thumbs.db",
Probably worth a bug here.

https://codereview.chromium.org/642403002/diff/500001/net/base/keygen_handler...
File net/base/keygen_handler_mac.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/keygen_handler...
net/base/keygen_handler_mac.cc:210: err = SecKeyCreatePair(keychain,
Probably a clang-format bug here.

It should have preferred to keep the original format (indent 4) rather than
aligning the first parameter.

https://codereview.chromium.org/642403002/diff/500001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/net_util.cc#ne...
net/base/net_util.cc:499: {224, 0, 0, 0, 3}};
Another weird formatting

https://codereview.chromium.org/642403002/diff/500001/net/base/priority_queue.h
File net/base/priority_queue.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/priority_queue...
net/base/priority_queue.h:34: // This section is up-front for Pointer only.
Seems like this was/is botched too?

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.cc
File net/base/sdch_manager.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.c...
net/base/sdch_manager.cc:143: .empty()) {
Bug in clang-format

It prioritized keeping a param at the same line higher than the cost of wrapping
the .empty()

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h
File net/base/sdch_manager.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h...
net/base/sdch_manager.h:143: // RESPONSE_TENTATIVE_SDCH in
../filter/sdch_filter.cc.
BUG in clang-format

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h...
net/base/sdch_manager.h:164: CACHE_DECODED = 92,  // No timing stats recorded.
bug in clang-format

[jkarlin, 2014-10-16 00:19:32]

Ryan:  Thanks for the fast feedback!  I've filed a couple of bugs, and defended
the formatter a few places where I think it did the reasonable thing.  I may
have to manually edit some code to make it look better with the formatter.

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
File net/base/directory_lister.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
net/base/directory_lister.h:51: enum SortType { NO_SORT, DATE, ALPHA_DIRS_FIRST,
FULL_PATH };
On 2014/10/11 02:04:46, Ryan Sleevi wrote:
> So, here's an example where the original code was clearer.
> 
> However, the reason why git cl format did this is because the documentation
was
> listed in 47-50. If we move that documentation inline, above each enum value,
it
> should "do the right thing"
> 
> I don't think it's worth filing a git cl format bug on it, because I think
it's
> the right behaviour for the general case (which is designed for inline values)

Acknowledged.

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc
File net/base/escape.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc#newc...
net/base/escape.cc:97: //   p  q  r  s  t  u  v  w  x  y  z  {  |  }  ~  <NBSP>
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> Seems like we should file a bug for this.

I'm not sure what clang-format could do better here.  Starting the comment at
the beginning of the line is weird (though useful in this case) and I believe
against the style guide.

Perhaps the thing to do is indent with /**/ like so:
//  ' ' !  "  #  $  %  &  '  (  )  *  +  ,  -  .  /
/**/ 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0,	  
/**/ 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0,

It's a bit strange, but at least indented.

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc#newc...
net/base/escape.cc:195: : ((third_byte >= 0xA6) && (third_byte <= 0xA9)))) {
On 2014/10/11 02:04:46, Ryan Sleevi wrote:
> This is something that annoys rvargas@ and pkasting@, but I think is the right
> thing.
> 
> Peter filed a bug on this, ISTR, as our current style guide (
> http://www.chromium.org/developers/coding-style#Code_Formatting ) says wrap
> _after_ operators (the existing code), not before (the new code)
> 
> I think clang-format had a config file flag added for this. Can you hunt it
down
> and, if so, add it to the chromium clang-format definition?

Hmm, the style guide specifically says binary operators.  The || is properly
wrapped here.  I'm not sure if the guide meant to include the ternary operator
or not.

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
File net/base/filename_util_internal.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
net/base/filename_util_internal.cc:142: "thumbs.db",
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> Probably worth a bug here.

Issued. https://code.google.com/p/chromium/issues/detail?id=423599

https://codereview.chromium.org/642403002/diff/500001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/net_util.cc#ne...
net/base/net_util.cc:499: {224, 0, 0, 0, 3}};
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> Another weird formatting

Issued: https://code.google.com/p/chromium/issues/detail?id=423596

https://codereview.chromium.org/642403002/diff/500001/net/base/priority_queue.h
File net/base/priority_queue.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/priority_queue...
net/base/priority_queue.h:34: // This section is up-front for Pointer only.
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> Seems like this was/is botched too?

This one is tricky.  It indented because the following line is a macro, so it
tried to line up with the macro.  Since the entire section is a macro, it seems
reasonable to leave as is.

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.cc
File net/base/sdch_manager.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.c...
net/base/sdch_manager.cc:143: .empty()) {
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> Bug in clang-format
> 
> It prioritized keeping a param at the same line higher than the cost of
wrapping
> the .empty()

Is that in the style guide anywhere?  I can't seem to find it.

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h
File net/base/sdch_manager.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h...
net/base/sdch_manager.h:143: // RESPONSE_TENTATIVE_SDCH in
../filter/sdch_filter.cc.
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> BUG in clang-format

You think?  It seems like it's asking a bit much to line up with the previous
line's comment within a comment.

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h...
net/base/sdch_manager.h:164: CACHE_DECODED = 92,  // No timing stats recorded.
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> bug in clang-format

ditto

[mmenke, 2014-10-16 00:25:53]

mmenke@chromium.org changed reviewers:
+ mmenke@chromium.org

[mmenke, 2014-10-16 00:25:54]

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.cc
File net/base/sdch_manager.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.c...
net/base/sdch_manager.cc:143: .empty()) {
On 2014/10/16 00:19:32, jkarlin wrote:
> On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > Bug in clang-format
> > 
> > It prioritized keeping a param at the same line higher than the cost of
> wrapping
> > the .empty()
> 
> Is that in the style guide anywhere?  I can't seem to find it.

Technically no, but preferring not to split an argument across lines if at all
possible is a Chromium style norm.

[Peter Kasting, 2014-10-30 20:20:58]

pkasting@chromium.org changed reviewers:
+ pkasting@chromium.org

[Peter Kasting, 2014-10-30 20:21:00]

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
File net/base/directory_lister.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
net/base/directory_lister.h:51: enum SortType { NO_SORT, DATE, ALPHA_DIRS_FIRST,
FULL_PATH };
On 2014/10/11 02:04:46, Ryan Sleevi wrote:
> So, here's an example where the original code was clearer.
> 
> However, the reason why git cl format did this is because the documentation
was
> listed in 47-50. If we move that documentation inline, above each enum value,
it
> should "do the right thing"
> 
> I don't think it's worth filing a git cl format bug on it, because I think
it's
> the right behaviour for the general case (which is designed for inline values)

Is there a case where enums are really better with all values collapsed
together?  I feel like for "enum X { ... }", the contents of the braces should
always be on their own lines, and would suggest filing a clang-format bug about
it.

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc
File net/base/escape.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc#newc...
net/base/escape.cc:195: : ((third_byte >= 0xA6) && (third_byte <= 0xA9)))) {
On 2014/10/16 00:19:32, jkarlin wrote:
> On 2014/10/11 02:04:46, Ryan Sleevi wrote:
> > This is something that annoys rvargas@ and pkasting@, but I think is the
right
> > thing.
> > 
> > Peter filed a bug on this, ISTR, as our current style guide (
> > http://www.chromium.org/developers/coding-style#Code_Formatting ) says wrap
> > _after_ operators (the existing code), not before (the new code)
> > 
> > I think clang-format had a config file flag added for this. Can you hunt it
> down
> > and, if so, add it to the chromium clang-format definition?
> 
> Hmm, the style guide specifically says binary operators.  The || is properly
> wrapped here.  I'm not sure if the guide meant to include the ternary operator
> or not.  

We meant only to exclude unary operators.  dcheng filed a bug about this one
yesterday on my behalf.

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.cc
File net/base/sdch_manager.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.c...
net/base/sdch_manager.cc:143: .empty()) {
On 2014/10/16 00:25:54, mmenke wrote:
> On 2014/10/16 00:19:32, jkarlin wrote:
> > On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > > Bug in clang-format
> > > 
> > > It prioritized keeping a param at the same line higher than the cost of
> > wrapping
> > > the .empty()
> > 
> > Is that in the style guide anywhere?  I can't seem to find it.
> 
> Technically no, but preferring not to split an argument across lines if at all
> possible is a Chromium style norm.

I would phrase this as "breaking at whitespace is pretty much always better than
breaking at non-whitespace such as around '.' or "->"."

[jkarlin, 2014-10-30 23:44:47]

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.cc
File net/base/sdch_manager.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.c...
net/base/sdch_manager.cc:143: .empty()) {
On 2014/10/30 20:20:59, Peter Kasting wrote:
> On 2014/10/16 00:25:54, mmenke wrote:
> > On 2014/10/16 00:19:32, jkarlin wrote:
> > > On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > > > Bug in clang-format
> > > > 
> > > > It prioritized keeping a param at the same line higher than the cost of
> > > wrapping
> > > > the .empty()
> > > 
> > > Is that in the style guide anywhere?  I can't seem to find it.
> > 
> > Technically no, but preferring not to split an argument across lines if at
all
> > possible is a Chromium style norm.
> 
> I would phrase this as "breaking at whitespace is pretty much always better
than
> breaking at non-whitespace such as around '.' or "->"."

Filed https://code.google.com/p/chromium/issues/detail?id=429029

[jkarlin, 2014-10-31 15:55:30]

My plan is to wait for the next clang-format roll, then I'll rebase, make
reasonable fixes by hand, reformat, and ask you to take another look.

[jkarlin, 2014-11-11 17:42:20]

Patchset #2 (id:710001) has been deleted

[jkarlin, 2014-11-11 17:43:17]

Patchset #2 (id:730001) has been deleted

[jkarlin, 2014-11-11 17:43:25]

Patchset #2 (id:750001) has been deleted

[jkarlin, 2014-11-11 19:01:44]

A rebase, an update on issues, and some fixes to code to work better with
clang-format.  Still need to get some issues fixed.

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
File net/base/directory_lister.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/directory_list...
net/base/directory_lister.h:51: enum SortType { NO_SORT, DATE, ALPHA_DIRS_FIRST,
FULL_PATH };
On 2014/10/30 20:20:59, Peter Kasting wrote:
> On 2014/10/11 02:04:46, Ryan Sleevi wrote:
> > So, here's an example where the original code was clearer.
> > 
> > However, the reason why git cl format did this is because the documentation
> was
> > listed in 47-50. If we move that documentation inline, above each enum
value,
> it
> > should "do the right thing"
> > 
> > I don't think it's worth filing a git cl format bug on it, because I think
> it's
> > the right behaviour for the general case (which is designed for inline
values)
> 
> Is there a case where enums are really better with all values collapsed
> together?  I feel like for "enum X { ... }", the contents of the braces should
> always be on their own lines, and would suggest filing a clang-format bug
about
> it.

I think this comes down to opinion, I personally like it shorter.

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc
File net/base/escape.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/escape.cc#newc...
net/base/escape.cc:97: //   p  q  r  s  t  u  v  w  x  y  z  {  |  }  ~  <NBSP>
On 2014/10/16 00:19:32, jkarlin wrote:
> On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > Seems like we should file a bug for this.
> 
> I'm not sure what clang-format could do better here.  Starting the comment at
> the beginning of the line is weird (though useful in this case) and I believe
> against the style guide.
> 
> Perhaps the thing to do is indent with /**/ like so:
> //  ' ' !  "  #  $  %  &  '  (  )  *  +  ,  -  .  /
> /**/ 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0,	  
> /**/ 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0,
> 
> It's a bit strange, but at least indented.

Fixed by wrapping with clang-format off

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
File net/base/filename_util_internal.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
net/base/filename_util_internal.cc:142: "thumbs.db",
On 2014/10/16 00:19:32, jkarlin wrote:
> On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > Probably worth a bug here.
> 
> Issued. https://code.google.com/p/chromium/issues/detail?id=423599

This one was supposed to have been fixed but isn't quite.  In discussion.

https://codereview.chromium.org/642403002/diff/500001/net/base/keygen_handler...
File net/base/keygen_handler_mac.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/keygen_handler...
net/base/keygen_handler_mac.cc:210: err = SecKeyCreatePair(keychain,
On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> Probably a clang-format bug here.
> 
> It should have preferred to keep the original format (indent 4) rather than
> aligning the first parameter.

Done.

https://codereview.chromium.org/642403002/diff/500001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/net_util.cc#ne...
net/base/net_util.cc:499: {224, 0, 0, 0, 3}};
On 2014/10/16 00:19:32, jkarlin wrote:
> On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > Another weird formatting
> 
> Issued: https://code.google.com/p/chromium/issues/detail?id=423596

Feedback on the bug was that this one really could go either way, which I agree
with.

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.cc
File net/base/sdch_manager.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.c...
net/base/sdch_manager.cc:143: .empty()) {
On 2014/10/30 23:44:47, jkarlin wrote:
> On 2014/10/30 20:20:59, Peter Kasting wrote:
> > On 2014/10/16 00:25:54, mmenke wrote:
> > > On 2014/10/16 00:19:32, jkarlin wrote:
> > > > On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > > > > Bug in clang-format
> > > > > 
> > > > > It prioritized keeping a param at the same line higher than the cost
of
> > > > wrapping
> > > > > the .empty()
> > > > 
> > > > Is that in the style guide anywhere?  I can't seem to find it.
> > > 
> > > Technically no, but preferring not to split an argument across lines if at
> all
> > > possible is a Chromium style norm.
> > 
> > I would phrase this as "breaking at whitespace is pretty much always better
> than
> > breaking at non-whitespace such as around '.' or "->"."
> 
> Filed https://code.google.com/p/chromium/issues/detail?id=429029

Still waiting on this one.

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h
File net/base/sdch_manager.h (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h...
net/base/sdch_manager.h:143: // RESPONSE_TENTATIVE_SDCH in
../filter/sdch_filter.cc.
On 2014/10/16 00:19:32, jkarlin wrote:
> On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > BUG in clang-format
> 
> You think?  It seems like it's asking a bit much to line up with the previous
> line's comment within a comment.

Fixed by adding whitespace.

https://codereview.chromium.org/642403002/diff/500001/net/base/sdch_manager.h...
net/base/sdch_manager.h:164: CACHE_DECODED = 92,  // No timing stats recorded.
On 2014/10/16 00:19:32, jkarlin wrote:
> On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > bug in clang-format
> 
> ditto 

It would be nice if it lined up better but it seems not worse to wrap it  with a
"clang-format off/on".

[jkarlin, 2014-11-11 19:34:48]

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
File net/base/filename_util_internal.cc (right):

https://codereview.chromium.org/642403002/diff/500001/net/base/filename_util_...
net/base/filename_util_internal.cc:142: "thumbs.db",
On 2014/11/11 19:01:43, jkarlin wrote:
> On 2014/10/16 00:19:32, jkarlin wrote:
> > On 2014/10/11 02:04:47, Ryan Sleevi wrote:
> > > Probably worth a bug here.
> > 
> > Issued. https://code.google.com/p/chromium/issues/detail?id=423599
> 
> This one was supposed to have been fixed but isn't quite.  In discussion.

Okay, fixed upstream in clang-format r221699 but won't make its way to chromium
for awhile.  In the meanwhile, I've updated the code to be more clang-format
friendly.

[mmenke, 2014-12-02 19:22:30]

A couple random comments, just glancing over some of the CL.  Only one thing
that seems like a clear bug (In net/base/net_log_util.cc)

https://codereview.chromium.org/642403002/diff/810001/net/base/data_url_unitt...
File net/base/data_url_unittest.cc (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/data_url_unitt...
net/base/data_url_unittest.cc:19: }
No linebreak at the end of a namespace is weird.  Maybe it's because there's no
"// namespace" comment?

https://codereview.chromium.org/642403002/diff/810001/net/base/filename_util_...
File net/base/filename_util_unittest.cc (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/filename_util_...
net/base/filename_util_unittest.cc:243: // Unfortunately, UTF8ToWide discards
invalid UTF8 input.
Hrm...wonder about the indentation of these comments.

https://codereview.chromium.org/642403002/diff/810001/net/base/keygen_handler...
File net/base/keygen_handler_mac.cc (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/keygen_handler...
net/base/keygen_handler_mac.cc:47: {0}};
This is sufficiently different that we may want to standardize on including a
bonus comma at the end of array initializer lists, though it doesn't work for
struct initializer lists, I assume.

https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
File net/base/net_info_source_list.h (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
net/base/net_info_source_list.h:23: NET_INFO_SOURCE(HTTP_CACHE, "httpCacheInfo",
1 << 8)
Not great...This is the kind of a place a no format header may be useful.

https://codereview.chromium.org/642403002/diff/810001/net/base/net_log_util.cc
File net/base/net_log_util.cc (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/net_log_util.c...
net/base/net_log_util.cc:56: ,
The placement of the comma here is really weird.

https://codereview.chromium.org/642403002/diff/810001/net/base/net_util_icu_u...
File net/base/net_util_icu_unittest.cc (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/net_util_icu_u...
net/base/net_util_icu_unittest.cc:65: true}},
This is pretty hideous, though not sure it's worth filing a bug about.

[Ryan Sleevi, 2014-12-02 19:38:45]

https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
File net/base/net_info_source_list.h (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
net/base/net_info_source_list.h:23: NET_INFO_SOURCE(HTTP_CACHE, "httpCacheInfo",
1 << 8)
On 2014/12/02 19:22:29, mmenke wrote:
> Not great...This is the kind of a place a no format header may be useful.

The original code was not style guide compliant.

[mmenke, 2014-12-02 19:44:50]

https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
File net/base/net_info_source_list.h (right):

https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
net/base/net_info_source_list.h:23: NET_INFO_SOURCE(HTTP_CACHE, "httpCacheInfo",
1 << 8)
On 2014/12/02 19:38:45, Ryan Sleevi wrote:
> On 2014/12/02 19:22:29, mmenke wrote:
> > Not great...This is the kind of a place a no format header may be useful.
> 
> The original code was not style guide compliant.

Does the style guide say anything about formatting macro usage?

[Ryan Sleevi, 2014-12-02 19:52:01]

On 2014/12/02 19:44:50, mmenke wrote:
>
https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
> File net/base/net_info_source_list.h (right):
> 
>
https://codereview.chromium.org/642403002/diff/810001/net/base/net_info_sourc...
> net/base/net_info_source_list.h:23: NET_INFO_SOURCE(HTTP_CACHE,
"httpCacheInfo",
> 1 << 8)
> On 2014/12/02 19:38:45, Ryan Sleevi wrote:
> > On 2014/12/02 19:22:29, mmenke wrote:
> > > Not great...This is the kind of a place a no format header may be useful.
> > 
> > The original code was not style guide compliant.
> 
> Does the style guide say anything about formatting macro usage?

Explicitly? No.
But C99 explicitly calls these as a term "function-like macro" (C99 6.10.3 p10),
and I see no reason why not to expect that
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Calls
applies

[mmenke, 2014-12-02 20:01:14]

"Sometimes arguments form a structure that is important for readability. In
those cases, feel free to format the arguments according to that
structure."  They also don't really mention horizontal whitespace, other
than the 4 space indent, curiously.

On Tue, Dec 2, 2014 at 2:52 PM, <rsleevi@chromium.org> wrote:

> On 2014/12/02 19:44:50, mmenke wrote:
>
> https://codereview.chromium.org/642403002/diff/810001/net/
> base/net_info_source_list.h
>
>> File net/base/net_info_source_list.h (right):
>>
>
>
> https://codereview.chromium.org/642403002/diff/810001/net/
> base/net_info_source_list.h#newcode23
>
>> net/base/net_info_source_list.h:23: NET_INFO_SOURCE(HTTP_CACHE,
>>
> "httpCacheInfo",
>
>> 1 << 8)
>> On 2014/12/02 19:38:45, Ryan Sleevi wrote:
>> > On 2014/12/02 19:22:29, mmenke wrote:
>> > > Not great...This is the kind of a place a no format header may be
>> useful.
>> >
>> > The original code was not style guide compliant.
>>
>
>  Does the style guide say anything about formatting macro usage?
>>
>
> Explicitly? No.
> But C99 explicitly calls these as a term "function-like macro" (C99 6.10.3
> p10),
> and I see no reason why not to expect that
> http://google-styleguide.googlecode.com/svn/trunk/
> cppguide.html#Function_Calls
> applies
>
>
>
> https://codereview.chromium.org/642403002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ryan Sleevi, 2014-12-02 20:04:33]

On 2014/12/02 20:01:14, mmenke wrote:
> "Sometimes arguments form a structure that is important for readability. In
> those cases, feel free to format the arguments according to that
> structure."  They also don't really mention horizontal whitespace, other
> than the 4 space indent, curiously.
> 

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Decl...

Default indentation is 2 spaces.
Wrapped parameters have a 4 space indent.

[mmenke, 2014-12-02 20:06:22]

On 2014/12/02 20:04:33, Ryan Sleevi wrote:
> On 2014/12/02 20:01:14, mmenke wrote:
> > "Sometimes arguments form a structure that is important for readability. In
> > those cases, feel free to format the arguments according to that
> > structure."  They also don't really mention horizontal whitespace, other
> > than the 4 space indent, curiously.
> > 
> 
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Decl...
> 
> Default indentation is 2 spaces.
> Wrapped parameters have a 4 space indent.

Right...I meant the space after a comma (Had forgotten the parameter in question
appeared alone on a line, as opposed to after a comma and some bonus spaces)

[Ryan Sleevi, 2014-12-02 20:06:37]

On 2014/12/02 20:04:33, Ryan Sleevi wrote:
> On 2014/12/02 20:01:14, mmenke wrote:
> > "Sometimes arguments form a structure that is important for readability. In
> > those cases, feel free to format the arguments according to that
> > structure."  They also don't really mention horizontal whitespace, other
> > than the 4 space indent, curiously.
> > 
> 
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Decl...
> 
> Default indentation is 2 spaces.
> Wrapped parameters have a 4 space indent.

and missed a bit from the paste:
All parameters should be aligned if possible.

(Which covers "Why is the second line indented to the first")

Yeah, we're style haggling, but that's a clang-format discussion without
haggling. What clang-format does is what I expect it to do, hence why I'm OK
with it doing it :)

[mmenke, 2014-12-02 20:07:00]

On 2014/12/02 20:06:22, mmenke wrote:
> On 2014/12/02 20:04:33, Ryan Sleevi wrote:
> > On 2014/12/02 20:01:14, mmenke wrote:
> > > "Sometimes arguments form a structure that is important for readability.
In
> > > those cases, feel free to format the arguments according to that
> > > structure."  They also don't really mention horizontal whitespace, other
> > > than the 4 space indent, curiously.
> > > 
> > 
> >
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Decl...
> > 
> > Default indentation is 2 spaces.
> > Wrapped parameters have a 4 space indent.
> 
> Right...I meant the space after a comma (Had forgotten the parameter in
question
> appeared alone on a line, as opposed to after a comma and some bonus spaces)

Ahh...right...on all lines except one, the question is indeed bonus spaces after
a comma.

[jkarlin, 2014-12-18 18:28:57]

Closing as this is now out of date and we don't have immediate plans to format
all of net/