[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-gnulib] new module proposal: strip

From: Bruno Haible
Subject: Re: [bug-gnulib] new module proposal: strip
Date: Fri, 1 Sep 2006 15:01:57 +0200
User-agent: KMail/1.9.1

Hi Davide,

[Re-added bug-gnulib to the CCs.]

Davide Angelocola wrote:
> On Thursday 31 August 2006 22:41, you wrote:
> > Thanks for the proposal. I think this functionality would be useful
> > in gnulib, but it needs a bit of polishing before your code can be
> > included:
> >
> > - The pretty standard name for this string manipulation is "trim",
> >   not "strip". See
> >     http://en.wikipedia.org/wiki/Trim_(programming)
> >     http://www.lisp.org/HyperSpec/Body/fun_string-tr_g-right-trim.html
> >     http://java.sun.com/j2se/1.5.0/docs/api/java/lang/String.html#trim()
> Done.
> > - The function should work correctly in multibyte locales. To achieve so,
> >   it should use the function iswspace() or iswblank() from <wctype.h>
> >   and the "mbiter.h" iterators for multibyte strings.
> Here I've troubles. I never used these functions. 
> > - Functions which do side-effects are generally better rewritten as
> >   functions without side-effects. I.e. instead of a function
> >       char * strip2(char *s, int how)
> >   I would prefer to see a function
> >       char * strip2(const char *s, int how)
> >   that allocates its result with "xalloc.h".
> I've used the strdup module. It is correct?
> Btw, you've to check the copyright and the license (GPL?) in the source 
> files.
> Best Regards,
> -- Davide Angelocola

About the function names: 'chug' and 'chomp' are not self-documenting
function names either. I'd propose trim_trailing and trim_leading.
(The Lisp name string-left-trim and string-right-trim come from a time
when people did not think about bidi.)

About mbiter.h: If you can not understand the mbiter.h functions, then
can you please point out what can be improved about its documentation?
I'd like to improve the comments in mbiter.h then.

Using strdup is ok, but you need to check that its return value is not
NULL. We often use xstrdup instead, which incorporates this extra check.

Also, trim2 does IMO not need to check against a NULL argument. Basic
string functions such as strlen and strcmp expect non-NULL arguments;
why should 'trim' then allow a NULL argument?

About the copyright header: Please use as starting year the year when
you wrote the first line of code still present in the current code.
You didn't start it in 1996, did you? - The copyright header is otherwise
ok. At some point, you will have to file copyright assignment papers for
your contributions to gnulib, giving the copyright ownership to the FSF.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]