Correct docs & deprecation message in std.complex.

[WebDrake]

Reference to std.format.format [sic!] should be to std.string.format.

I've also taken the opportunity to slightly clean up the module's import statements and some whitespace errors (someone in the past had left a bunch of tabs lying around:-)

[quickfur]

Hmm. I think referring to FormatSpec makes the documentation needlessly obscure. Why not just refer the user to std.string.format instead? After all, if he's looking at toString, probably the most appropriate substitute is std.string.format.

It would take a while before a newbie even begins to understand just what FormatSpec is and how to actually use it in practice.

[WebDrake]

I made that tweak because FormatSpec is what's actually used in the unittest examples. But happy to switch to std.string.format if that's preferred.

[quickfur]

Really? I thought the unittest examples used std.string.format, it's the function body that uses FormatSpec. IMO, pointing users to std.string.format is better, because they can immediately use it in their code. Pointing them to FormatSpec would just make them scratch their heads going, "FormatSpec? What's that? How do I use it?" And the documentation for FormatSpec isn't exactly the most helpful in terms of end-user code. I found it really obtuse when I was first learning D; it was only later that I understood what it was used for.

Just my opinion, though. I'll let the Phobos devs give the official position.

[WebDrake]

The point is that the relevant version of toString expects a FormatSpec input, not a format input.

[quickfur]

Ah, sorry I didn't get your point the first time.

My take on that is that users should be encouraged to use std.string.format instead of dealing directly with toString. It's a bit weird in the deprecation message for toString to point them to FormatSpec if the former expects a FormatSpec argument, as though FormatSpec somehow replaces toString.

My point is, instead of:

auto c = Complex(...);
string s = c.toString(FormatSpec(...));

they should use:

auto c = Complex(...);
string s = std.string.format("%.02f", c);

[WebDrake]

Yea, you're right, I didn't get your meaning properly before.

I don't know how the old, deprecated toString was meant to be used -- presumably as a consequence of using some other external function, rather than calling it directly? The new toString that expects a FormatSpec input should certainly never be called itself but is accessed indirectly via std.string.format.

So, if that's true, the deprecation message should be in reference to what the old external function was, which std.string.format should replace; and not in respect to a direct call to toString.

For now, I've tweaked the docs with a compromise solution: the deprecation message requests the use of FormatSpec but the documentation for toString refers (as it should) to std.string.format (and restores the import statement in the unittest which is there for documentation purposes).

[monarchdodra]

I think version unittest imports are generally a terrible idea. I've seen the release broken 3 [edit]times[/edit] because of this, one of which in an actual release.

Please have each individual unittest that require it (it's what 1?) include what it needs individually.

[WebDrake]

OK, this is turning into more hassle than it's worth -- I think I've messed up too many things here. I'm going to push a complete fresh patch with all feedback taken into account.

[quickfur]

On Wed, Nov 27, 2013 at 10:36:52AM -0800, Joseph Rushton Wakeling wrote:

OK, this is turning into more hassle than it's worth -- I think I've
messed up too many things here. I'm going to push a complete fresh
patch with all feedback taken into account.
[...]

git rebase -i is your friend. :)

T

Those who don't understand Unix are condemned to reinvent it, poorly.

[WebDrake]

Already using it :-)

[monarchdodra]

unittest import apart, lgtm. please squash though :)

[WebDrake]

OK, squashed and (I think) properly corrected. The import std.string : format statements are back inside the unittests where they belong, the references to std.format.format have been correctly corrected to std.string.format.

And some whitespace has been cleaned up, because in tweaking one of the doc statements I realized the file had a bunch of tabs left over from a previous author ;-)

[quickfur]

That would be me again. :-/ I seriously need to just switch wholesale to sw=4 ts=4 et in my vim config. In my own D code, I rely on vim modelines at the bottom of source files to set up the tabbing correctly (I have older code that use 8-character tabs, which would look awful with 4-char tabs, and worse after I've edited them with expandtabs since it would look OK in vim but wrongly indented everywhere else -- that's why I haven't put this in my .vimrc yet), but Phobos code doesn't have the modelines, and I keep forgetting to set things up manually when submitting pulls.

[WebDrake]

My .vimrc, if it's interesting to you:

filetype plugin indent on
syntax on
set sts=0 sw=4 ts=4
autocmd FileType c,cpp,d,java,php autocmd BufWritePre <buffer> :%s/\s\+$//e
autocmd FileType c,cpp,d,java :setl cinoptions=(0,u0,U0
autocmd FileType c,cpp set noet
autocmd FileType d,java set et

Gives me mixed tab-space indent in C/C++, space indent in D and Java (not that Java is something I work with, but one should Be Prepared:-). Note that I use the ctab plugin by Michael Geddes: http://www.vim.org/scripts/script.php?script_id=231

[monarchdodra]

That would be me again. :-/

We've all been there. No-one will any stones at you ;)

[WebDrake]

Thanks for the quick merge, and sorry for the noise/confusion :-)