Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add documentation examples for formatValue() #2296

Merged
merged 1 commit into from
Jul 4, 2014
Merged

Add documentation examples for formatValue() #2296

merged 1 commit into from
Jul 4, 2014

Conversation

JLTastet
Copy link
Contributor

@JLTastet JLTastet commented Jul 2, 2014

Add example showing how to forward format specifiers.
Add example highlighting the similarity with formattedWrite(), and
comparing their use.

@@ -2655,6 +2655,44 @@ const string toString();
$(LI Otherwise, they are formatted like $(D Type(field1, filed2, ...)).))

Otherwise, are formatted just as their type name.

Examples:
<dl>formatValue allows to reuse existing format specifiers :
Copy link
Member

Choose a reason for hiding this comment

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

Stray <dl> and extraneous space before the colon.

@JakobOvrum
Copy link
Member

It looks like these two could be documented unit tests.

@JLTastet
Copy link
Contributor Author

JLTastet commented Jul 2, 2014

Will documented unit tests appear in the documentation ? I know ddox has this feature, but I did only test the changes using the "old" layout (without ddox).
I'll remove the extra space before the colon. The <dl> was used to correct the vertical spacing, but it may be unnecessary with the new layout. I'll remove it too.

@JakobOvrum
Copy link
Member

Will documented unit tests appear in the documentation ? I know ddox has this feature, but I did only test the changes using the "old" layout (without ddox).

Yes, it is a feature of plain DDoc as well. Put the example explanation text in the unit test's DDoc comment.

I'll remove the extra space before the colon. The <dl> was used to correct the vertical spacing, but it may be unnecessary with the new layout. I'll remove it too.

Don't ever put HTML directly into the documentation. Some places in the language documentation still do this, but it's being phased out.

@JLTastet
Copy link
Contributor Author

JLTastet commented Jul 2, 2014

Updated.

@JakobOvrum
Copy link
Member

LGTM.

Note that it's possible to do more succinct one-line documentation comments using /// comment.

@@ -2697,6 +2697,52 @@ if (is(T == class) && !is(T == enum))
}
}

/**
formatValue allows to reuse existing format specifiers:
Copy link
Member

Choose a reason for hiding this comment

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

$(D formatValue)? Also ///

Copy link
Collaborator

Choose a reason for hiding this comment

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

$(D formatValue)?

Technically not needed, as formatValue is the function being documented, so it is already always implicitly $D'ed. You can opt out by writing _formatValue in the documentation body, and it will appear as plain formatValue in the text.

Also ///

As long as it's a for a function, or documenting something that actually explains something, IMO, there's nothing wrong with a "proper" documentation header. /// really feels like it should be reserved for trivial getters or, enum values, or whatnot.

That said, I really wish we'd promote /++ +/ more than /** */. Unless you plan to backport to C, then + is always at least as good as *. End rent.

Copy link
Member

Choose a reason for hiding this comment

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

Technically not needed, as formatValue is the function being documented, so it is already always implicitly $D'ed.

Is it? I think it's printed in green, but not in monotype. But the DDoc blocks lower had other D symbols too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it? I think it's printed in green, but not in monotype. But the DDoc blocks lower had other D symbols too.

I'm actually unsure. I know Andrei has made some "useless" comments when function names were being $D'ed in documentation. But no, I don't actually know what it does. IMO, there's nothing wrong with tagging with $D anyways. I just thought I'd mention it. Anybody know what happens exactly to the names of documented functions in DDoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested and if I omit $(D formatValue), the function name is still displayed in green but not in boldface, while $(D formattedWrite) still appears in black and boldface.
I also replaced /** */ with /++ +/. I will update in a few minutes.
BTW, I am not quite used to the GitHub workflow. Is it better to merge all the commits (rebase -i), amend the first one (probably not) to keep the history clean or just leave them all ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The rule of thumb is: Minimize your commits, but have each commit contain only one logical change. Of course, if things get complicated, one huge commit is fine. The only thing is we try to avoid merging commits that are "Oops", "fix", "typo" etc...

During the process proper, you may or may not merge your commits as you go. Both ways have their advantages. Personally, I just like it when I make a comment, to be pinged when the comment is addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then it might be better to merge them all, here. The choice of comment delimiters is secondary.

@JLTastet
Copy link
Contributor Author

JLTastet commented Jul 2, 2014

Yes, I know it. I actually tried to mimic the coding style that was used elsewhere in std.format, but I can still change it.

Add example showing how to forward format specifiers.
Add example highlighting the similarity with formattedWrite(), and
comparing their use.

Change /** */ for /// and add $(D formatValue)

Replace /** */ with /++ +/
@DmitryOlshansky
Copy link
Member

Nice work! LGTM

@DmitryOlshansky
Copy link
Member

Auto-merge toggled on

DmitryOlshansky added a commit that referenced this pull request Jul 4, 2014
Add documentation examples for formatValue()
@DmitryOlshansky DmitryOlshansky merged commit 6883e0e into dlang:master Jul 4, 2014
9rnsr pushed a commit to 9rnsr/phobos that referenced this pull request Jul 7, 2014
…ample

Add documentation examples for formatValue()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants