bytestring icon indicating copy to clipboard operation
bytestring copied to clipboard

Upper case hex builders missing

Open hth313 opened this issue 7 years ago • 10 comments

In the documentation this is mentioned "Note that there is no support for using upper-case characters. Please contact the maintainer, if your application cannot work without hexadecimal encodings that use upper-case characters."

Now I am in the position where I really must have upper case hexadecimal output and hopefully this is the correct place to contact the maintainer.

hth313 avatar Feb 09 '17 18:02 hth313

A PR would be accepted. It should be straightforward based on the current code.

dcoutts avatar Feb 13 '17 09:02 dcoutts

I have mixed feelings about this, especially given that #117 adds 25 (!) new public functions. Could we possibly expose somehow more stuff from Data.ByteString.Builder.Prim to make such things easier to express on the client's side?

Bodigrim avatar Oct 08 '20 18:10 Bodigrim

I'm tempted to close this due to lack of activity or interest.

Bodigrim avatar Feb 16 '22 00:02 Bodigrim

The reason that there are so many new functions is that I tried to provide corresponding functions to what already existed. I think the original implementation has too many variants, but it seems bad to remove them.

This was a while back, but the one that takes a 64-value with a field size is very useful and it did not exist in the existing implementation.

I am mainly interested in the fixed width unsigned variants word8HexUpperFixed, word16HexUpperFixed, word32HexUpperFixed, word64HexUpperFixed and word64HexFixedUpperWidth.

Of these I only use word8HexUpperFixed, word16HexUpperFixed and word64HexFixedUpperWidth.

I think word64HexFixedWidth and word64HexFixedUpperWidth are very useful and the lower case is not present in the current implementation.

If you can accept that the PR provides upper case variants that does not fully implement all lower-case variants, then I am happy with that. I would suggest adding either the two Width variants and maybe also the four additional WordN variants. That is if you can accept that they do not fully match the lower case functions, which are way too many.

hth313 avatar Feb 16 '22 01:02 hth313

@hth313 what's the difference between word64HexUpperFixed and word64HexFixedUpperWidth? What width are we talking about?

Bodigrim avatar Feb 16 '22 23:02 Bodigrim

@Bodigrim The word64HexFixedUpperWidth function can output something narrower than the type that holds it. The word64HexFixedUpper will always output a 64 bit value.

Emitting something that is narrower than the type is very useful when a value (of some size up to 64 bits) is represented internally with a 64 bit type. 64 bits is enough (today) for most low level architecture needs and the widest fixed width type supported. The idea is that some values are actually not 64 bits, but the value is still kept in the same type that is wide enough (Word64). The width is provided from the environment in some way. Side note, I tend to normalize values that are kept like this, e.g. sign extending them based on the width.

The word64HexFixedUpper function exists because there are functions for the other word sizes. It may feel odd to omit that one. Using word64HexFixedUpperWidth with a width argument of 16 will do the same thing as word64HexFixedUpper.

I have no strong preference on whether word64HexUpperFixed should exist or not, it is a matter of taste. I think the Width functions are very useful.

hth313 avatar Feb 17 '22 04:02 hth313

I think adding word64HexFixedUpperWidth could be a reasonable extension. @sjakobi what's your opinion?

Bodigrim avatar Feb 17 '22 20:02 Bodigrim

Sounds good to me.

sjakobi avatar Feb 18 '22 16:02 sjakobi

@hth PR is welcome.

Bodigrim avatar Mar 16 '22 20:03 Bodigrim

@Bodigrim Yes, I understand. I am in the middle of some release work and will look at in about 2-4 weeks.

hth313 avatar Mar 16 '22 20:03 hth313