OVERRIDE line break.

11 views
Skip to first unread message

Chad Faragher

unread,
Mar 24, 2011, 12:02:43 PM3/24/11
to chromium-dev
What is the approved style for 80 column breaking of lines like this?
  virtual void OnMouseReleased(const views::MouseEvent& event, bool
canceled) OVERRIDE;

It reaches 80 cols after the V in OVERRIDE.

Here are some ways I can think of:
  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled)
                               OVERRIDE;
  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled)
      OVERRIDE;
  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled
                               ) OVERRIDE;
  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled
      ) OVERRIDE;
  virtual void OnMouseReleased(const views::MouseEvent& event,
                               bool canceled) OVERRIDE;

One could imagine this applying to other keywords like const, or the
pure virtual specifier = 0.

Scott Violet

unread,
Mar 24, 2011, 12:07:12 PM3/24/11
to wy...@chromium.org, chromium-dev
The style guide says that for const it should be on the same line as
the last parameter, eg:

 virtual void OnMouseReleased(const views::MouseEvent& event,

                               bool canceled) const;

not:

 virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled)

const;

I think we should treat OVERRIDE the same way.

-Scott

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
>    http://groups.google.com/a/chromium.org/group/chromium-dev
>

Mark Mentovai

unread,
Mar 24, 2011, 12:10:24 PM3/24/11
to wy...@chromium.org, chromium-dev
Chad Faragher wrote:
What is the approved style for 80 column breaking of lines like this?
  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled) OVERRIDE;

There are a few approved options. It depends on the situation, the code around you, and sometimes personal (or reviewer) preference.
 
It reaches 80 cols after the V in OVERRIDE.

Here are some ways I can think of:
  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled)
                               OVERRIDE;

Probably not, because there’s no reason for OVERRIDE to be lined up with the arguments. It’s not inside the parentheses, it’s not an argument.

  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled)
      OVERRIDE;

This is acceptable.

  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled
                               ) OVERRIDE;
  virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled
      ) OVERRIDE;

No to both of these. I wouldn’t ever put the closing ) anywhere other than right up against the final argument.

  virtual void OnMouseReleased(const views::MouseEvent& event,
                               bool canceled) OVERRIDE;

This would be my preferred choice in this case.

William Chan (陈智昌)

unread,
Mar 24, 2011, 12:20:05 PM3/24/11
to ma...@chromium.org, wy...@chromium.org, chromium-dev
I agree with Mark, although I think I'd have a strong preference for the last option, related to what Scott said above. It looks weird to me to have a keyword that modifies the member function on a line by itself. I think it looks weird to me because we don't do that with const, as Scott noted.

--

Chad Faragher

unread,
Mar 24, 2011, 12:34:59 PM3/24/11
to Scott Violet, chromium-dev
On Thu, Mar 24, 2011 at 12:07 PM, Scott Violet <s...@chromium.org> wrote:
> The style guide says that for const  it should be on the same line as
> the last parameter, eg:
>
>  virtual void OnMouseReleased(const views::MouseEvent& event,
>                                bool canceled) const;

OK, cool. Thanks, Scott.

Peter Kasting

unread,
Mar 24, 2011, 1:34:59 PM3/24/11
to s...@chromium.org, wy...@chromium.org, chromium-dev
On Thu, Mar 24, 2011 at 9:07 AM, Scott Violet <s...@chromium.org> wrote:
The style guide says that for const  it should be on the same line as
the last parameter, eg:

 virtual void OnMouseReleased(const views::MouseEvent& event,
                               bool canceled) const;

not:

 virtual void OnMouseReleased(const views::MouseEvent& event, bool canceled)
   const;

I think we should treat OVERRIDE the same way.

I have been instructing people to treat OVERRIDE exactly like const in my reviews, so I agree with this.

While we're here, a reminder note about function declaration style: if you linebreak at all, you must not have more than one arg per line (except for rare cases where adjacent arguments are closely related, e.g. "int x, int y"), and if you can't line every arg up with the '(', you should instead break after the '(' and indent each arg 4.  Function calls, OTOH, have great flexibility; you can do a number of different things.

PK
Reply all
Reply to author
Forward
0 new messages