Page MenuHomePhabricator

[clang-tidy] update links to Google Code Style in docs
ClosedPublic

Authored by omtcyf0 on Feb 25 2016, 4:41 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

omtcyf0 updated this revision to Diff 49039.Feb 25 2016, 4:41 AM
omtcyf0 retitled this revision from to [clang-tidy] update links to Google Code Style in docs.
omtcyf0 updated this object.
omtcyf0 added reviewers: cfe-commits, alexfh.
omtcyf0 removed a reviewer: cfe-commits.
omtcyf0 added a subscriber: cfe-commits.
hokein added inline comments.
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49039)

This should be in one line?

omtcyf0 added inline comments.Feb 25 2016, 5:10 AM
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49039)

It is in html; breakline here is because of 80 chars per line limit.

alexfh added inline comments.Feb 25 2016, 5:53 AM
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49039)

HTML? This is rst, not html.

Anyway, the link shouldn't be split in two lines, otherwise it won't work. The 80-characters limit is not a hard limit, it can be violated, when you have long links or filenames.

Also, please create diffs with the full context when sending patches for review (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

alexfh requested changes to this revision.Feb 25 2016, 5:53 AM
alexfh edited edge metadata.
This revision now requires changes to proceed.Feb 25 2016, 5:53 AM
omtcyf0 updated this revision to Diff 49048.Feb 25 2016, 6:05 AM
omtcyf0 edited edge metadata.

Removed the breakline.

omtcyf0 added inline comments.Feb 25 2016, 6:07 AM
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49048)

It is rst. I meant generated HTML docs there.

IIRC rst is similar to markdown there -- the generated output won't have newline there unless there are 2 spaces at the end of line or a blank line.

alexfh added inline comments.Feb 25 2016, 6:17 AM
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49048)

One of the good properties of markdown and rst (though rst is worse in this regard) is that these formats are readable and usable as a plain text (unlike HTML, for example). Breaking the link makes the link unusable in the text mode (at least, not with vim in a terminal).

alexfh accepted this revision.Feb 25 2016, 6:20 AM
alexfh edited edge metadata.

LG. Thanks!

This revision is now accepted and ready to land.Feb 25 2016, 6:20 AM

Hajian, if you have no concerns, could you commit the patch? Thanks!

omtcyf0 added inline comments.Feb 25 2016, 6:21 AM
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49048)

Okay, won't do that again :)

hokein accepted this revision.Feb 25 2016, 6:23 AM
hokein edited edge metadata.

LGTM. I will commit the patch for you @omtcyf0.

Thank you very much, @hokein!

hokein added inline comments.Feb 25 2016, 6:26 AM
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49048)

IIRC rst is similar to markdown there -- the generated output won't have newline there unless there are 2 spaces at the end of line or a blank line.

Indeed, the generated html will not have a new line, but the link will be broken.

omtcyf0 added inline comments.Feb 25 2016, 6:35 AM
docs/clang-tidy/checks/readability-named-parameter.rst
13 ↗(On Diff #49048)

Uh, you're right!

Sorry for arguing before, I thought it won't be; thanks for pointing out!

This revision was automatically updated to reflect the committed changes.