Re: g_build_path/filename

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj gtk org> writes:

> On 8 Sep 2001, Owen Taylor wrote:
> 
> > The following patch adds what I think is a pretty
> > important function we missed: 
> >  
> >  gchar *g_build_filename (const gchar *first_element, ...);
> > 
> > And as a "free" side effect of implementation:
> > 
> >  gchar *g_build_path (const gchar *separator, ...);
> > 
> > Which is not as important but fairly useful.
> > 
> > The reasons for adding g_build_filename() are:
> > 
> >  - It gets used a lot (there are ~10 places in GTK+ that 
> >    use g_strconcat ( ... G_DIR_SEPARATOR_S ... ) 
> > 
> >  - Doing it yourself is ugly on Unix, but fatal on
> >    Win32, where \\ in a filename is illegal.
> > 
> > This is basically a replacement for g_concat_dir_and_file()
> > in gnome-libs, which was one of the more commonly used
> > convenience functions from gnome-libs.
> > 
> > I think it would be worth slipping this in, even at this
> > point ... I've discussed it with various people in the 
> > past, and got favorable reactions but never actually got around 
> > to writing anything.
> 
> i think they are usefull enough to get in.
> however, it'd be good if they also were directly deployed, so
> we get rid of the current places which do this on their own.

I wanted to hold off on making a big GTK+/pango patch until I was 
sure that we were going to add the API. :-)
 
> as for the patch, i consider the first!=NULL stuff a bit hackish in
> g_build_pathv(). instead i'd rather see you make it handle the
> public API:
> 
> gchar *	g_build_filename (const gchar *first_element,
>                           ...);
> gchar *	g_build_path	 (const gchar *separator,
>                           const gchar *first_element,
>                           ...);

OK, done. My intent was to keep the prototypes what I first wrote
down figuring those were the most natural prototypes, but 
adding the first_element does make it clear that:

 g_build_path (":");

Isn't allowed, so is perhaps a bit better on those grounds.
It also is a little simpler and a little more symmetric.

> and not allow first_element==NULL, since there's little point
> in supporting g_build_filename(NULL)=="".

There doesn't seem to me to be any value to me to not _supporting_
g_build_filename(NULL) = "". 

Yes, it's not useful, but it keeps the operation more consistent
and logical to support it. I think g_return_if_fail() statements
should be there to say "you are doing something wrong" not to
say "you are doing something pointless."

And I could just barely imagine someone wanting to use it with
g_build_path().

 path = g_build_path (":", dir_a ? dir_a : dir_b, dir_b);

Or something like that.

> also, the docs need a comment that these functions do _not_
> return an absolute path.

OK, added comment to g_build_filename():

 * No attempt is made to force the resulting filename to be an absolute
 * path. If the first element is a relative path, the result will
 * be a relative path. 

Regards,
                                        Owen