[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