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

Lack of String.strip #4444

Closed
vicuna opened this Issue Nov 11, 2007 · 4 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link
Collaborator

vicuna commented Nov 11, 2007

Original bug ID: 4444
Reporter: Snark
Assigned to: @xclerc
Status: closed (set by @xavierleroy on 2012-09-25T18:07:21Z)
Resolution: fixed
Priority: normal
Severity: feature
Version: 3.10.0
Fixed in version: 3.12.1+dev
Category: ~DO NOT USE (was: OCaml general)

Bug description

Here is an implementation, hopefully not too buggy :
let strip str =
let len = String.length str
and is_space char = (char = ' ' || char = '\t') in
let start = ref 0
and stop = ref (len - 1)
and seen_non_space = ref false in
for ii = 0 to len - 1 do
if is_space str.[ii] then
(if not !seen_non_space then
start := ii + 1)
else
(seen_non_space := true;
stop := ii)
done;
String.sub str !start (!stop - !start +1)

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 11, 2007

Comment author: Snark

The function is_space_char should probably have a || char = '\n' so it's possible to strip a full line.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 13, 2007

Comment author: thelema

Is the intent to trim whitespace off the ends of a string? i.e. " foo " -> "foo"? If so, I might write it like this (not tested):

let trim s =
let rec skip_space dir i =
if s.[i] = ' ' || s.[i] = '\t' (* throws Invalid_argument )
then skip_space dir (i+dir)
else i in
try
let start = skip_space 1 0
and end = skip_space -1 (String.length s - 1) in
String.sub str start (end-start+1)
with Invalid_argument _ -> "" (
out of bounds searching for non-space *)

I admit to cheating a bit on the bounds handling, but this seems a more caml-esque implementation. As to the question of whether this should be in the standard library, I'd vote yes, but I'm for a heavier stdlib than OCaml has even now.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 13, 2007

Comment author: Snark

I propose pushing the test on what is a whitespace in a function like I did -- and let's not forget to cut '\n' too.

Your solution looks better : you only go on the whitespaces while mine goes on the whole string. Notice though that it doesn't work because you use the 'end' keyword as a variable and miss some ( ) here and there.

Amended solution :

let trim s =
let is_space c = (c = ' ' || c = '\t' || c = '\n') in
let rec skip_space dir i =
if is_space s.[i] (* throws Invalid_argument *)
then skip_space dir (i+dir)
else i
in
try (
let start = skip_space 1 0
and stop = skip_space (-1) ((String.length s) - 1) in
String.sub s start (stop -start+1)
) with Invalid_argument _ -> "";;

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 13, 2007

Comment author: thelema

Even with a perfect implementation, the question quickly comes "does this function belong in the stdlib?"

The two best arguments against inclusion are: 1) having to maintain more code as part of the core OCaml distribution, and 2) Those who need this function are probably doing quick & dirty parsing and using this function to clean up improperly cut strings.

Considering the first point, imagine a more shared ownership of maintenance responsibility. The OCaml community has resources (time, programming energy) to contribute to finding and fixing bugs. At the moment, those at INRIA seem to allow us outsiders to only find & report bugs, but want to control strongly what makes it into the ocaml distribution. This makes some sense for the core compiler, as its architecture must remain solid in order for them to use it as a base for their research. And users of OCaml come and go, while those at INRIA will continue to have a hand in code they contribute for a long time. OTOH, the stdlib seems like a fine place for the community to take part in making OCaml more friendly, as the barrier of entry is much lower.

As to the second point, I find only one fault with it: users still want this functionality, even if there exists a better way.

@vicuna vicuna closed this Sep 25, 2012

@vicuna vicuna added the feature-wish label Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.