Skip to content

Conversation

denis-sh
Copy link
Contributor

@andralex
Copy link
Member

tests are failing

@denis-sh
Copy link
Contributor Author

tests are failing

This is D1 pull. Autotester tests D2 only. As you can see it tries to merge this into D-Programming-Language:master instead of D-Programming-Language:phobos-1.x and obviously fails.

@denis-sh
Copy link
Contributor Author

Ping.

@denis-sh
Copy link
Contributor Author

@WalterBright, what's wrong with this?

{
import std.c.stddef; // wchar_t

extern (Windows) void* LocalFree(void*);
Copy link
Member

Choose a reason for hiding this comment

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

Could extern(Windows)be factored out and used as a scope here?

@andralex
Copy link
Member

Could please someone with more Windows knowledge take over reviewing this diff? Thanks.

@ghost
Copy link

ghost commented Sep 28, 2012

First things first, where are the unit tests?

@ghost ghost assigned WalterBright Sep 29, 2012
@denis-sh
Copy link
Contributor Author

denis-sh commented Oct 4, 2012

To @andralex:

Could extern(Windows)be factored out and used as a scope here?

Factored out.


To @AndrejMitrovic:

First things first, where are the unit tests?

I have no experience in such kind of unittests. Show me such unittests, please (which build program, run it with arguments and test its output).

@denis-sh
Copy link
Contributor Author

denis-sh commented Oct 4, 2012

By the way, this pull works correct on my windows system.

@denis-sh
Copy link
Contributor Author

Ping.

@ghost
Copy link

ghost commented Oct 28, 2012

I have no experience in such kind of unittests. Show me such unittests, please (which build program, run it with arguments and test its output).

@denis-sh: My bad, I haven't realized this is D1, not D2.

@alexrp
Copy link
Contributor

alexrp commented Nov 1, 2012

Since this is already in druntime for D2, I see no reason to not merge this.

alexrp pushed a commit that referenced this pull request Nov 1, 2012
@alexrp alexrp merged commit 5469df0 into dlang:phobos-1.x Nov 1, 2012
@denis-sh
Copy link
Contributor Author

denis-sh commented Nov 2, 2012

Finally. Thanks a lot!

@WalterBright
Copy link
Member

Doesn't work. Fails the test:

wc wc.d
lines words bytes file
Error: ↑

--- errorlevel 1

where wc.d is:

import std.file;

int main (char[][] args)
{
int w_total;
int l_total;
int c_total;

printf ("   lines   words   bytes file\n");
foreach (char[] arg; args[1 .. args.length])
{
    char[] input;
    int w_cnt, l_cnt, c_cnt;
    int inword;

    input = cast(char[])std.file.read(arg);

    foreach (char c; input)
    {
        if (c == '\n')
            ++l_cnt;
        if (c != ' ')
        {
            if (!inword)
            {
                inword = 1;
                ++w_cnt;
            }
        }
        else
            inword = 0;
        ++c_cnt;
    }
    printf ("%8lu%8lu%8lu %.*s\n", l_cnt, w_cnt, c_cnt, arg.length, arg.ptr);
    l_total += l_cnt;
    w_total += w_cnt;
    c_total += c_cnt;
}
if (args.length > 2)
{
    printf ("--------------------------------------\n%8lu%8lu%8lu total",
        l_total, w_total, c_total);
}
return 0;

}

@WalterBright
Copy link
Member

hmm, love the bad auto-formatting of github!

@WalterBright
Copy link
Member

reverted

WalterBright added a commit that referenced this pull request Nov 20, 2012
@ghost
Copy link

ghost commented Nov 20, 2012

Use ```D for syntax highlighting of D code.

Also, didn't I mention this pull needed unittests? :)

@denis-sh
Copy link
Contributor Author

@WalterBright:

hmm, love the bad auto-formatting of github!

Disagree, its rather good but you have to read how to use it once (a link at top-right corner of text box you write to). And you can use "Preview" button to see results.

@denis-sh
Copy link
Contributor Author

Looks like my "code duplication from druntime" message foces everybody to not look at code at all (me included). The code uses alloca in a nested function. Fixed in #962.

@denis-sh denis-sh mentioned this pull request Nov 20, 2012
@denis-sh
Copy link
Contributor Author

Wrong base branch in #961, superseded by #962.

@denis-sh
Copy link
Contributor Author

To @AndrejMitrovic:
Your comment #789 (comment)

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