<< Back to previous view

[CLJ-826] Include drop, take, butlast from clojure.contrib.string (1.2) in clojure.string 1.3 Created: 08/Aug/11  Updated: 02/Dec/11  Resolved: 02/Dec/11

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Backlog
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Arthur Edelstein Assignee: Unassigned
Resolution: Declined Votes: 3
Labels: None

Attachments: File clj-826-add-take-drop-butlast.diff     File clj-826-add-take-drop-butlast-v3.diff     File clj-826-add-take-drop-butlast-with-versions.diff    
Patch: Code and Test
Approval: Test
Waiting On: Matthew Lee Hinman

 Description   

From clojure.contrib.string 1.2, I have found myself using drop, take,
and butlast. (These are more than just wrappers for String/substring,
because they behave nicely when indices exceed the string length.) I
like these methods in part because they match the behavior of
corresponding sequence methods, but have better performance. This
makes optimizing (when needed) easier.



 Comments   
Comment by Matthew Lee Hinman [ 08/Aug/11 1:22 PM ]

Attached patch for adding the methods.

Comment by Matthew Lee Hinman [ 08/Aug/11 6:28 PM ]

I intentionally left out the {:added "_"} from the vars since I have no idea when this will be added.

This (as expected) causes this test to fail:
[java] FAIL in (public-vars-with-docstrings-have-added) (metadata.clj:42)
[java] expected: (= [] (remove (comp :added meta) public-vars-with-docstrings))
[java] actual: (not (= [] (#'clojure.string/butlast #'clojure.string/drop #'clojure.string/take)))

Which is easily remedied. If desired, let me know what version this will be added to and I'll submit a new patch with the :added metadata included.

Comment by Matthew Lee Hinman [ 12/Nov/11 4:29 PM ]

I have attached a patch with "1.4" as the added version now that 1.3 has been released.

Comment by Chouser [ 18/Nov/11 10:29 PM ]

The algorithms, docs, and tests look good. I think the functions would be better if they used clojure.core/subs like the rest of clojure.string does, instead of .substring. This is more idiomatic Clojure and allows you to avoid hinting the String arg.

When a new patch is uploaded, please remember to set the Patch field of this ticket to "Code and Test" and the Approval field to "Test". Hopefully this will lead to faster screening.

Comment by Matthew Lee Hinman [ 21/Nov/11 12:50 PM ]

Attached a patch that uses subs instead of .substring (clj-826-add-take-drop-butlast-v3.diff).

Comment by Stuart Halloway [ 02/Dec/11 9:14 AM ]

These fns were left out intentionally. Feel free to propose a contrib home for them, though.

Generated at Sun May 26 01:27:35 CDT 2013 using JIRA 4.4#649-r158309.