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

Adds split functions for sequence types including lists, vectors, seqs and strings #1083

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

Conversation

Izaakwltn
Copy link
Collaborator

This adds splitting by single-character delimiters

Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

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

in the future it might be nice for this to work on any sequence type with any Eq element type

library/string.lisp Outdated Show resolved Hide resolved
library/string.lisp Outdated Show resolved Hide resolved
@Izaakwltn
Copy link
Collaborator Author

A few updates on the most recent commit:

  • Added the splittable class which has a method split [coalton-library/split]
  • Added splittable instances for List Vector and Seq containing elements with Eq instances.
  • Renamed the existing string:split to string:bisect
  • The new string:split calls split:split on a list of chars.

@Izaakwltn Izaakwltn changed the title Added delim-split for strings Added Splittable class and split function for sequences including strings (with delimiter) Apr 5, 2024
@stylewarning
Copy link
Member

I'm not 100% convinced this is a class we should include. My main reason is that it seems a little contrived: we wanted something generic so we forced it to be. I think the true generic aspect of something that splittable isn't that it's splittable, but rather that elements have a natural ordering (i.e., they can be indexed) and each element can be accessed in finite time.

My recommendation for the direction of this PR is to create a function split in each relevant module (no need to give distinguishing names), forgo the type class, and we can ponder in the future some nice classes to capture such sequential properties.

@Izaakwltn Izaakwltn changed the title Added Splittable class and split function for sequences including strings (with delimiter) Adds split functions for sequence types including lists, vectors, seqs and strings Apr 8, 2024
@Izaakwltn
Copy link
Collaborator Author

Now this merely defines a function split for string, seq, list, and vector.

@Izaakwltn Izaakwltn force-pushed the delim-split branch 3 times, most recently from 5f71597 to 8242ff2 Compare May 10, 2024 20:38
@Izaakwltn
Copy link
Collaborator Author

No urgency, but ready for review whenever the next PR/Issue cleanup happens.

5/10:

  • rebased onto main so it's still compatible
  • cleaned up formatting

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.

None yet

2 participants