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

[Nick Treleaven]

On 26/01/2012 05:21, Lex Trotman wrote:
> On Thu, Jan 26, 2012 at 3:41 PM, Matthew Brush<mbrush at codebrainz.ca>  wrote:
>> On 01/25/2012 06:45 PM, Lex Trotman wrote:
>>>
>>> [...]
>>> Hi Matthew,
>>>
> [...]
>>> 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.
>
> You raise a good point, documentation and awareness of utils

I think reading utils.h (and utils.c for specific details) is a 
reasonable expectation. Often using autocompletion helps if you know the 
start of the function name.

> resources.  ATM the only documentation produced is for functions in
> the API, needs a version for the utils and any other generally useful
> bits.  Can doxygen distinguish two sets of doc comments in the one
> file so we can make a utils documentation set as well as the plugin
> API?

I would guess doxygen can't do that. Also it might be less clear as to 
what's in the plugin API and what's not.

> [...]
>>> 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).
>>
>
> You'll have to ask Enrico (I think) why the win API is used and why
> builds on windows are synchronous, thankfully most of this was done
> before I arrived and I only have a vague awareness of the reasons.
> OTOH I don't know that I like your chances of fixing windows Glib and
> then it will be in a version we get to a ways in the future so the win
> interface will have to stay for some time.

I'm pretty sure the glib spawn functions do not work, other projects had 
this problem too. I think it is a design flaw rather than implementation 
detail, but feel free to research this.

FWIW I think utils_spawn_async() is a disaster - it tries to combine 
async and sync parameters in one function, they are fundamentally different.

If we implement async spawn on Windows (like SciTEWin::ExecuteOne()) 
then the utils_spawn_async() parameters would no longer make sense.

Maybe we should deprecate it now because of these issues (it's in the 
plugin API).

> [...]
>>> 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.

Actually utils_make_filename() is really just g_strconcat without one 
separator argument. I don't mind if we remove it.

To reply to remaining points from earlier message:

On 26/01/2012 02:45, Lex Trotman wrote:
 > utils_slist_remove_next() - local static used one place, agree no
 > reason to exist

I decided to remove it. I thought it might get used elsewhere, but it 
didn't.

 > utils_string_replace() - probably should be static, only used several
 > times in utils itself

This was used in editor.c but the code got rewritten differently. I'm 
not sure that it's worth making it static as this will trigger a rebuild 
of src/*.o and we might use it sometime.

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

Good point.