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 unchop to array and unchop, repeat_str and mul to string #3155

Merged
merged 2 commits into from
May 28, 2019

Conversation

dipinhora
Copy link
Contributor

@dipinhora dipinhora commented May 17, 2019

This commit adds the following:

  • Update chop in string and array to allow is to work correctly
    after an unchop
  • unchop to both array and string to allow the un-chopping of a
    chopped string or array
  • repeat_str and mul to string to allow easy repeating of
    strings

@dipinhora
Copy link
Contributor Author

dipinhora commented May 17, 2019

I originally needed the unchop function, but then I wanted an easy way to repeat strings as part of writing tests for unchop. This PR has been created as a draft as the unchop name isn't exactly great.

Let me know if the preference is that some or all of these changes go through the RFC process instead.

This commit adds the following:

* Update `chop` in string and array to allow `is` to work correctly
  after an `unchop`
* `unchop` to both array and string to allow the un-chopping of a
  chopped string or array
* `repeat_str` and `mul` to string to allow easy repeating of
  strings
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.

Seems reasonable to me. And useful - I can think of some things I've worked on in the past where I'd have liked this to exist.

Does it also make sense to have an unchop_val as well?

@dipinhora
Copy link
Contributor Author

Not sure about unchop_val. The use case I had in mind for unchop required needing to get the original mutable array or string back to what it was before it was chopped. For val strings and arrays, you can keep a reference to the original if needed when you trim since both the original and the trimmed ones are immutable.

@jemc
Copy link
Member

jemc commented May 21, 2019

I was thinking about use cases where you chop a buffer to get two iso buffers, send them to separate actors to mutate them, then convert them to val so they can be shared, and then you want to eventually put the pieces back together into a single val.

However, I suppose it can wait until someone is writing a real-world program that actually has this need.

@dipinhora
Copy link
Contributor Author

I would accomplish the same thing by assembling them back together before converting to val to share.

However, if you think it would be useful and regularly needed, I can add it in. Let me know.

@jemc
Copy link
Member

jemc commented May 21, 2019

We can wait until the need arises in a real application.

fun mul(num: USize): String iso^ =>
"""
Returns a copy of the string repeated `num` times with an optional
separator added inbetween repeats.
Copy link
Contributor

Choose a reason for hiding this comment

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

From the looks of it, this docstring appears to be incorrect, as mul does not accept an argument for a separator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed.

@SeanTAllen
Copy link
Member

Is this ready for final review @dipinhora ?

@dipinhora
Copy link
Contributor Author

@SeanTAllen yes, marking as ready for review now.

@dipinhora dipinhora marked this pull request as ready for review May 28, 2019 16:08
@SeanTAllen SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label May 28, 2019
@SeanTAllen
Copy link
Member

@dipinhora can you add release notes to this PR to be included in the "happening very soon" release?

@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label May 28, 2019
@SeanTAllen SeanTAllen merged commit 861ccda into ponylang:master May 28, 2019
@dipinhora
Copy link
Contributor Author

release notes (possibly):

Adds unchop to both array and string to allow the the original string or array to be recovered by combining the two pieces from a previous chop.
Adds repeat_str and mul to string to allow easy repeating of strings (such as 'abc' * 6 to get abcabcabcabcabcabc or 'abc'.repeat_str(6, ', ') to get abc, abc, abc, abc, abc, abc)

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 changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants