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