[Geany-devel] encoding combo boxes bug - utility functions

Matthew Brush mbrush at xxxxx
Thu Jan 26 04:41:34 UTC 2012


On 01/25/2012 06:45 PM, Lex Trotman wrote:
> [...]
> Hi Matthew,
>
> While in general I agree with you, your examples are of mixed
> accuracy, see below:
>
>> [1] Just at a very quick scan through utils.c, things like
>
> utils_slist_remove_next() - local static used one place, agree no
> reason to exist
>
> utils_is_uri() - good utility function, well named
>

Indeed well named and probably a little clearer that `strstr(uri, "://") 
!= NULL` but that's probably what I'd write if I didn't know Geany had 
it's own function for this, or I'd use g_uri_parse_scheme() or something.

> utils_string_replace() - probably should be static, only used several
> times in utils itself
>
> utils_spawn_async() - I think was used more than one place in the
> past, also hides the messy #ifdef windoze which is good
>

If the GLIB functions don't work (ie. on win32), we should send a bug 
report/patch upstream, just as we'd do with Scintilla if we found an 
obvious bug. We shouldn't have our own fixed implementation, IMO (unless 
it does something else I'm not aware of, or upstream refused the fix).

> utils_build_path() - g_build_filename() has better portability
> semantics, should replace utils_build_path()
>

Yep, why I listed it.

> utils_make_filename() - reasonable utility function, probably should
> be used in more places where filename.ext concat is done explicitly
>

I never would've thought to use this function over g_strjoin() and 
g_build_filename() or something. Having this seems weird to me, despite 
it being mildly useful and having good doc comment, since the name isn't 
great and the two things it does are so commonly available separately 
already in GLIB.

But anyway, I made this list in 2 minutes scanning utils.c, so possibly 
not the best examples.

Cheers,
Matthew Brush



More information about the Devel mailing list