Skip to content

Doc formatting fixes for DMD PR 4745#3722

Merged
Hackerpilot merged 6 commits intodlang:masterfrom
Abscissa:for-issue-14413
Oct 17, 2015
Merged

Doc formatting fixes for DMD PR 4745#3722
Hackerpilot merged 6 commits intodlang:masterfrom
Abscissa:for-issue-14413

Conversation

@Abscissa
Copy link
Contributor

Doc formatting fixes needed for dlang/dmd#4745

etc/c/curl.d Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually work? I thought macros had to be self-contained within a comment block

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, right. Fixing.

BTW, are those even supposed to be doc comments? Doesn't seem like it, but I don't really know this module very well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, you're right. These definitely do not refer to maxredirs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That surprised me because it does work for /// doc comments. Adjacent /// get merged before processing. At least it seems to work fine based on the results of dlang/druntime#1408 over at http://dtest.thecybershadow.net/artifact/website-fa397df5041e797d4ecdefd0231aa6ae97f9ea34-52af7a31c124ce2ec1c988269811b9fb/web/library-prerelease/core/cpuid/prefer_athlon.html

@quickfur
Copy link
Member

LGTM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? It's necessary to have no whitespace here? Wouldn't that break a lot of existing ddocs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, that one's unrelated to dlang/dmd#4745, but I tossed it in because I noticed the "ditto" is showing up in the generated docs ( http://dtest.thecybershadow.net/artifact/website-1d618eaba1a0a418cbe85589a97781591809b3fa-5a06ed4863a1b8ed5de4d7e83de8bae8/web/library-prerelease/std/container/array/array.remove_back.html , near the bottom of the page, in the last "Complexity" section).

But now that I look at the test results for this PR ( http://dtest.thecybershadow.net/results/efe54d2753dcaecdd25f95ec40132bc0d15ff2c1/a53d6f3607852ae15236d10fae4dbacdc86a24fb/ ), removing the space doesn't appear to have made any difference, so I'll revert this change.

@CyberShadow
Copy link
Member

Something odd with the std.socket changes.

std/socket.d Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened here? These lines seem to have been spuriously reordered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh. My text editor has an (overly) convenient key combo for "swap lines". Must've hit it my mistake without noticing. Will fix.

@CyberShadow
Copy link
Member

We should still change those etc.c.curl DDoc comments to be regular comments.

@Hackerpilot
Copy link
Contributor

Seems reasonable. Pressing the button.

@Hackerpilot
Copy link
Contributor

Auto-merge toggled on

@CyberShadow
Copy link
Member

@Abscissa Please add a comment after updating a PR, as reviewers aren't otherwise notified of new commits.

@CyberShadow
Copy link
Member

Looks good. Would have been nice to squash the commits though.

Hackerpilot added a commit that referenced this pull request Oct 17, 2015
Doc formatting fixes for DMD PR 4745
@Hackerpilot Hackerpilot merged commit 5a2d290 into dlang:master Oct 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants