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

Ehance ddoc for std.array.split. #2817

Merged
2 commits merged into from Jan 2, 2015
Merged

Ehance ddoc for std.array.split. #2817

2 commits merged into from Jan 2, 2015

Conversation

mdparker
Copy link
Member

This started with fixing a simple typo, but I decided to go ahead and go all out. If I'm on the right track with this, I'll do more.

I've added an extra paragraph, appropriate DDOC tags (Examples, Params, Returns), and changed the template type parameters to match the type parameters for std.algorithm.splitter. I also renamed the delimiter parameter to 'sep' to match std.array.join which follows this (std.algorthim.splitter calls it 's' instead).

I'm not sure about the description of the returned array. Should it be called an 'array of ranges'?

Finally, I think the version of the template that has the 'alias isTerminator' needs to be documented separately, but I haven't figured out yet what that template alias is doing.

An array containing the divided parts of $(D r).

See_Also:
$(XREF algorithm, splitter) for the lazy version of this operator.
Copy link
Member

Choose a reason for hiding this comment

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

Function, not operator

Copy link
Member Author

Choose a reason for hiding this comment

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

That was in the original text and I overlooked it. Updated.

@ghost
Copy link

ghost commented Dec 31, 2014

Can we avoid using single characters for the range? It's even easier to read the docs if we use range as the parameter name.

@ghost
Copy link

ghost commented Dec 31, 2014

Or use rng if we're saving characters, not sure..

@ghost ghost self-assigned this Dec 31, 2014
@mdparker
Copy link
Member Author

mdparker commented Jan 1, 2015

Changing 'r' to 'range' is fine with me. We ought to be consistent about it, though, since 'r' is used in a lot of places.

@ghost
Copy link

ghost commented Jan 1, 2015

Could you squash the first two commits together? (the ones introducing and then changing operator->function)

@mdparker
Copy link
Member Author

mdparker commented Jan 1, 2015

Sure, if you give me some hints on how to do it.

@mdparker
Copy link
Member Author

mdparker commented Jan 1, 2015

Oh, nevermind. I see "squash" is a real thing. I'll figure this out.

@ghost
Copy link

ghost commented Jan 1, 2015

Ah sorry, I haven't realized you're new to Git!

Typically you would use the git interactive mode to edit the commits. For example:

# edit the last two commits
$ git rebase -i HEAD~2

And then you usually get a vi editor (or notepad probably on Windows) with:

pick <sha> Ehance ddoc for std.array.split.
pick <sha> Minor edit of the See_Also.

You can change the second pick to either s (squash) or f (fixup). The terminology is a little loose, typically we say squash when we really mean fixup. The only difference between the two is: When using squash, you'll get a new commit message that is a combination of the two commit messages. You can still edit that message entirely, of course.

In your case you can simply use a fixup.

@mdparker
Copy link
Member Author

mdparker commented Jan 1, 2015

So after googling how to squash and how to move around the editor in git-bash (which appears to be vim), I think I've managed it somehow.

@mdparker
Copy link
Member Author

mdparker commented Jan 1, 2015

I've been happily using git for a few years now. Just pushing and pulling for the most part. This is my second PR ever.

@ghost
Copy link

ghost commented Jan 1, 2015

Good job! Done it without my help. :P

@ghost
Copy link

ghost commented Jan 1, 2015

Auto-merge toggled on

@yebblies
Copy link
Member

yebblies commented Jan 2, 2015

Auto-merge toggled on

This will work a lot better now that I've approved him on the autotester.

ghost pushed a commit that referenced this pull request Jan 2, 2015
Ehance ddoc for std.array.split.
@ghost ghost merged commit 9509581 into dlang:master Jan 2, 2015
@ghost
Copy link

ghost commented Jan 2, 2015

This will work a lot better now that I've approved him on the autotester.

lol! I didn't see the status anywhere so I just replace the typical autotester URL with /2817 and it worked.

Perhaps there should be a message here in the status bar "Author not approved for auto-testing" or something. :)

@ghost ghost removed their assignment Apr 28, 2015
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants