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

Add ability to get iso array from an iso string. #2889

Merged
merged 1 commit into from
Oct 3, 2018

Conversation

dipinhora
Copy link
Contributor

No description provided.

Copy link
Member

@jemc jemc left a comment

Choose a reason for hiding this comment

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

I think this qualifies for "principle of least surprise" for bypassing the RFC process.

Any others think differently before I merge?

@EpicEric
Copy link
Contributor

EpicEric commented Sep 29, 2018

I am not a fan of the method with iso in its name. Perhaps something like data or something better?

@jemc
Copy link
Member

jemc commented Oct 1, 2018

@EpicEric - I tend to agree that using iso in a method name is not ideal, but note that this PR is just following the precedent set by the from_array/from_iso_array methods.

new val from_array(data: Array[U8] val) =>
"""
Create a string from an array, reusing the underlying data pointer.
"""
_size = data.size()
_alloc = data.space()
_ptr = data.cpointer()._unsafe()
new iso from_iso_array(data: Array[U8] iso) =>
"""
Create a string from an array, reusing the underlying data pointer
"""
_size = data.size()
_alloc = data.space()
_ptr = (consume data).cpointer()._unsafe()
if _alloc > _size then
_set(_size, 0)
end

@Theodus Theodus added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Oct 3, 2018
@jemc jemc merged commit f41316a into ponylang:master Oct 3, 2018
ponylang-main added a commit that referenced this pull request Oct 3, 2018
@SeanTAllen
Copy link
Member

@dipinhora can you add release notes for this?

@SeanTAllen SeanTAllen mentioned this pull request Oct 11, 2018
@dipinhora
Copy link
Contributor Author

@SeanTAllen is the commit message not adequate for what the release notes require?

@SeanTAllen
Copy link
Member

@dipinhora I'm not seeing a commit message.

@dipinhora
Copy link
Contributor Author

@SeanTAllen The commit message was the one line title only (Add ability to get iso array from an iso string.) but I suppose that's not going to be sufficient for the release notes.

How's the following?

It is now possible to consume a string iso to get the underlying Array[U8] iso for when it is important to convert a string to an array but retain the ability to modify it afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - added Automatically add "Added" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants