Added split operations to IsSequence, EqSequence

[richsmith92]

No description provided.

[snoyberg]

Would you mind adding a comment about whether the separators are retained or discarded? A concrete example would be useful. At least I personally get confused about this whenever I use split-style functions.

[snoyberg]

In addition to the documentation comments, would you be able to add a test case for this, to codify the semantics?

[richsmith92]

I'll add following (from ByteString docs):

The resulting components do not contain the separators. Two adjacent separators result in an empty component in the output.

And will work on test case as well

[gregwebs]

I find the naming of these unintuitive (even if it may be consistent with some of the underlying libraries).
Our current naming scheme uses on to signify a function for groupOn (in that case it is returning an Ord). So splitOn and splitWhen would mean the same thing by that convention

I think split from the patch can be renamed to splitBy or splitAt or splitElem.
Perhaps splitOn from the patch can be renamed to splitSeq ?

[MaxGabriel]

IIRC the different splitting operators have slightly different edge case behavior (IIRC it was how they handle being given an empty string as the separator, or how splitting on a string consisting of just the separator works). I don't remember for sure but it might be worth double checking.

[richsmith92]

Considering the naming, Data.List.Split, ByteString and Text provide three different naming schemes. I followed the first, except I added lacking split :: Element seq -> seq -> [seq]. I find myself using split most of time, especially for CSV/TSV parsing, so I judged it deserves the shortest name.

I think the most intuitive short name for seq -> seq -> [seq] must be splitSeq. Following the same logic, I could rename split to splitElem, but then nice short split name is unused.

To summarize:

splitWhen is good name already
splitOn should be renamed to splitSeq (better ideas?)
split should either stay same or be renamed to splitElem

[gregwebs]

@w3rs I agree with your explanation. Do you use classy-prelude? One option is to re-export splitElem as split in classy-prelude. I am still wondering if there is something more intuitive: splitAt captures it for me, except that at often refers to integer indexed.

[richsmith92]

Do you use classy-prelude? One option is to re-export splitElem as split in classy-prelude.

@gregwebs yes, it makes sense.

splitAt captures it for me, except that at often refers to integer indexed.

This name is actually already taken with this purpose:

splitAt :: Index seq -> seq -> (seq, seq)

[gregwebs]

I am favoring splitElem, but I will go with consensus (or better ideas that come up).

[richsmith92]

I pushed the new commit with improvements. It also has new intercalate function for IsSequence (I needed it as inverse for splitSeq)

[gregwebs]

Looking good! If you want a shorter name, you could use splitEq instead of splitElem

[richsmith92]

@gregwebs I think I can bear a couple extra chars after all.

[snoyberg]

Looks good to me, I'm happy to merge thanks for all the improvements.

Can you both confirm that this is ready to go?

[richsmith92]

@snoyberg since we couldn't find much better names than splitElem and splitSeq, I'm fine with what we have.

[kbillings]

You cannot use '' with >. It produces invalid documentation. You must wrap the statement like:

@
'splitSeq' (singleton sep) === 'splitElem' sep
@

[richsmith92]

@kbillings good catch! I had no success with stack haddock and forgot to ask others to review the docs. Will remove single quotes from lines with >

[gregwebs]

thanks a lot!