Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Mar 17, 2015

@nomad-software
Copy link
Contributor

I'd perhaps add a template constraint to isHexLiteral and maybe rename:

static bool isHex(T)(in T hexData) if (isSomeString!T)

and shouldn't this go in std.string along with isNumeric?

@ghost
Copy link
Author

ghost commented Mar 17, 2015

std.ascii already has isHexDigit, in isHexLiteral the point is that whites chars are valid.

return false;
i += isH;
}
return !(i & 1) & (i > 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this? It already returns early if it's not a hex literal.

edit:

Nevermind, I see now it's not that simple. I still think the code is seriously confusing. bool is not supposed to be treated like an 8-bit integer.

@ghost
Copy link
Author

ghost commented Mar 17, 2015

the last return statement is not about isWhite or isHexDigit it's about the parity of the hex digits count (i & 1), confere with https://github.com/D-Programming-Language/dmd/blob/master/src/lexer.c#L1186

@JakobOvrum
Copy link
Contributor

I like the compile-time templates, but I don't think we should commit to the runtime functions yet. They're pretty inefficient and are hard-coded to using the GC. If we make them public, it's only a matter of time before someone complains that they don't accept ranges, don't accept an output range, aren't @nogc, aren't lazy; heck, here I am, already complaining :)

@ghost
Copy link
Author

ghost commented Mar 17, 2015

The idea is to use the same private template for the compile time and run time versions. I wait for more comments about this. The initial idea of Walter is to replace a single feature, but i recognize that i've been a bit overzealous in the commit.

@JakobOvrum
Copy link
Contributor

The idea is to use the same private template for the compile time and run time versions.

And I'm fine with that, but I think making the runtime functions public is a mistake, as they are too specific.

$(D wchar) or $(D dchar).
*/
@property @safe nothrow pure
auto hexString(string hexData, C = char)()
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be value templates, not function templates:

template hexString(string hexData, C = char)
    if (hexData.isHexLiteral && isSomeChar!C)
{
    immutable hexString = hexStrImpl!C(hexData);
}

The function $(D isHexLiteral) checks the consistency of a string for the
functions $(D hexString) and $(D hexBytes).
The result is only true if the input string is composed of ASCII white
characters (\f\n\r\t\v) or hexadecimal digits (regarless of the case).
Copy link
Contributor

Choose a reason for hiding this comment

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

regardless

+ added ddoc example for isHexLiteral
@ghost ghost closed this Mar 23, 2015
@WalterBright
Copy link
Member

Why was this closed?

@dnadlinger
Copy link
Contributor

It seems like @bbasile has deleted his fork of Phobos.

@WalterBright
Copy link
Member

Seems a shame. Looks like I'll adopt and resurrect it myself.

@ghost
Copy link
Author

ghost commented Apr 1, 2015

I've deleted the source fork because i forgot to create a brach for the patch. Later i didnt re-fork it because i thought the initial PR would fall into a black hole.

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
Development

Successfully merging this pull request may close these issues.

6 participants