Fix Issue 15720 - iota(long.max, long.min, step) does not work properly #5396
Conversation
std/range/package.d
Outdated
| iota(0, 5).map!(i => format("%s ", i)).copy(stdout.lockingTextWriter()); | ||
| writeln(); | ||
| } | ||
| --- |
There was a problem hiding this comment.
Also took the opportunity to unindent the doc.
There was a problem hiding this comment.
I don't see the point, as Phobos uses tons of different doc styles all over the place.
Honestly I really, really don't want this to become a topic of conversation in the community. Can you drop this from the PR?
There was a problem hiding this comment.
The original doc style (and the prevalent style in this particular file) has the doc flush with the symbol being documented. I see reason to take the opportunity to consolidate style a bit, and as a contributor to the docs I find the gratuitous indent unpleasant.
There was a problem hiding this comment.
Also took the opportunity to unindent the doc.
If it's not too much trouble, please do such changes as separate commits in the future.
|
Needs a test case |
andralex
force-pushed
the
15720
branch
2 times, most recently
from
ec91b5e
to
13e579b
Compare
|
ah, forgot to paste the test |
|
Auto-merge toggled on |
std/range/package.d
Outdated
| end = The value that serves as the stopping criterion. This value is not | ||
| included in the range. | ||
| step = The value to add to the current value at each iteration. | ||
| Creates a range of values that span the given starting and stopping values. |
There was a problem hiding this comment.
This mixes whitespace changes and non-whitespace changes. Please never do this.
Even with ?w=1 it's not completely clean because some text has been rewrapped.
There was a problem hiding this comment.
ah ok that's a good point
std/range/package.d
Outdated
| @@ -5553,6 +5552,12 @@ nothrow @nogc @safe unittest | |||
| auto t1 = iota(0, 10, 2); | |||
| auto t2 = iota(1, 1, 0); | |||
| //float overloads use std.conv.to so can't be @nogc or nothrow | |||
| alias ssize_t = Unsigned!size_t; | |||
There was a problem hiding this comment.
When adding tests for a bug, it's nice to add a comment mentioning the Bugzilla issue number.
|
Auto-merge toggled off |
|
Disabling auto-merge for above feedback, but feel free to re-enable it if you feel these aren't worth addressing. |
|
fixed thx for the reviews |
I think a stray commit got in. |
|
ugh not sure what happened, lemme fumble around a bit more |
|
Not sure how to reopen this, so I created #5397 |
|
If you close a PR temporarily, make sure you reopen before you push. GitHub doesn't allow reopening PRs if you've pushed to the PR branch while it was closed. (If you've already pushed, i is still possible to undo the push, reopen, and redo the push, but at that point it's probably easier to open a new PR.) |
Forgot conversion to unsigned in length. I suppose there are a couple more bugs in extreme cases.