On 26/01/2012 05:21, Lex Trotman wrote:
On Thu, Jan 26, 2012 at 3:41 PM, Matthew Brush<mbr...@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.
_______________________________________________
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel

Reply via email to