Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added unicode generators [PR was resubmitted] #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arcusfelis
Copy link
Contributor

Add improper_list/2, maybe_improper_list/2 generators.
PR #62 is an old version of this request.

Add improper_list/2, maybe_improper_list/2 generators.
@motiejus
Copy link
Contributor

I would expect unicode_binary(Size) to be up to Size bytes long, instead of codepoints. That makes more sense for database applications, for example. However, it is so much more difficult to do it that way. I have done an attempt, but it has 1 (only 1) problem: it doesn't shrink. Manolis Papadakis promised to look at it when he has time, so it's pending. :) As I couldn't figure out why it doesn't shrink, I created a brain-dead simple implementation which does the purpose (but isn't as efficient as I want it to be).

Also, you could add unit-like tests, and which ensures it shrinks properly. proper_tests.erl is the place to look at.

Just updated the original ticket (#48).

@dcartt
Copy link

dcartt commented Feb 23, 2013

Some generated unicode is failing an is_instance check. For instance, this fails for every value generated (always on the first generated value):

?FORALL(T, unicode_binary(), proper_types:is_instance(T, unicode_binary())).

unicode_characters() fails too. unicode_char() and unicode_string() work as expected, correctly flagging all values generated as instances.

I hope unicode gets added to the main branch! Seems pretty important to have these types available.

@mmzeeman
Copy link

mmzeeman commented Mar 5, 2014

Any news regarding the unicode generator?

@motiejus
Copy link
Contributor

motiejus commented Mar 5, 2014

After some thinking I think unicode generator does not belong to PropEr core. We should have proper_contrib or so?

Reason: unicode generator can use only "official" PropEr APIs and does not need any changes in internals.

Thoughts?

@mmzeeman
Copy link

mmzeeman commented Mar 5, 2014

Thanks for the quick response.

That sounds ok too. As long as I can create random utf8 binaries for types like these:

-type diff_op() :: delete | equal | insert.
-type diff() :: {diff_op(), unicode:unicode_binary()}.
-type diffs() :: list(diff()).

unicode_binary() is defined in the unicode module as an alias for binary().

After some thinking I think unicode generator does not belong to PropEr core. We should have proper_contrib or so?

Reason: unicode generator can use only "official" PropEr APIs and does not need any changes in internals.

Thoughts?


Reply to this email directly or view it on GitHub.

@kostis
Copy link
Collaborator

kostis commented Mar 6, 2014

I do not have a strong opinion on this. What @motiejus points out makes sense, but on the other hand it's very convenient to have it built-in and not have to know about and download another package.

The only requirements we have if this is to become part of PropEr is that it is as robust as possible (in particular, it should shrink properly) and it comes with tests and documentation. If I understand the comments here, the current pull request does not have these properties, but I have not played with it.

manopapad added a commit that referenced this pull request Mar 30, 2014
@StoneCypher
Copy link

This ticket should be closed, as it's fixed in 3d211d6

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