Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
They don't need to call C directly, GI just needs to generate the appropriate adapters in the typelib. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379458442
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
> To be fair to GI (what! me saying that!) most languages do not have variadic > functions, so the GI has nothing to translate into. Python: ```python def foo(*args): for a in args: print(a) ``` JavaScript: ```js function foo(...args) { for (let a of args) print(a); } ``` Ruby: ```ruby def foo(*args) for a in args print a end end ``` Vala: ```vala void foo(...) { foreach (var a in va_list()) print(a); } ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379458090
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
> > Of course it's for convenience. Imagine writing the following line of code > > without using any functions with variadic arguments: > printf("%s is %d", name, age); ``` std::cout<
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
> I'd argue that, for future APIs, we don't export variadic ones. These are > generally just for convinience. But plugins can easily do their own string > formatting and give us the result. Of course it's for convenience. Imagine writing the following line of code without using any functions with variadic arguments: ```c printf("%s is %d", name, age); ``` Even if you did use one variadic function (ex. `g_strdup_printf`), it's still 3 lines of code and a potential source for leaking memory. > This part of GI is not buggy. Ok, it's not buggy, it's just a missing feature which makes it impossible to export all of the C API to other languages, thus requiring any bindings to C be incomplete or rewriting the C API to be less convenient. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379455887
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
GI requires actual symbols that are exported by the library. I'd argue that, for future APIs, we don't export variadic ones. These are generally just for convinience. But plugins can easily do their own string formatting and give us the result. This part of GI is not buggy. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379376035
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
@b4n true, or having the script generate: ```c /* GTK-DOC comment with annotations here */ #define msgwin_compiler_add_string(s) msgwin_compiler_add("%s", s) ``` Or whatever the buggy GI can actually understand, instead of all the manual duplication. My worry is that each time we add hacks to the public API to work around bugs in GI we make the API worse (ex. more confusing, more maintenance, etc) for the common use case. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379365080
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
@codebrainz not relly, because you need not to have the called function interpret the string as a format string, otherwise something like `Geany.Msgwin.msg_add("100%data")` would happily crash without any reasonable mean of fixing it -- e.g. short of the caller knowing he has to escape `%`s but that they otherwise don't mean anything. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379270198
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
IMO it would be better to fix [the script](https://github.com/geany/geany/blob/master/scripts/gen-api-gtkdoc.py) to automatically generate the needed signatures than duplicate every API function that has varargs with a non-vararg version. This should be relatively easy, at least where the varargs are format strings (ie. have the script replace `...` with `const char*`). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379129907
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
Closed #1748 via 1611e3f94960a821ced0af9b99052afa6ade2111. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#event-1559580772
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
@kugel- what I don't like with the `@close` method is that it marks the PR as closes rather than merged. IIUC, what using GitHub's buttons, it does work though, so I imagine it might be possible somehow to do that manually too -- but I might be dreaming. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-379057448
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
@kugel- pushed 1 commit. daa9800 fixup! msgwin: improve doxygen comments -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1748/files/e032739afaca7f58f9a6563d743474f2f74499f0..daa9800098fae65ce1d68641e93ca8d0c87dc8a1
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
BTW, if I knew a way to let GitHub think a PR is merged when manually squasing it, I could fix those nitpickings myself when merging… anybody aware of such a possibility? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-371997727
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
b4n requested changes on this pull request. Sorry… Otherwise looks good, just need updating the `@since`, squashing and we're good to merge. > @@ -488,6 +519,32 @@ void msgwin_status_add(const gchar *format, ...) } } +/** + * Logs a formatted status message *without* setting the status bar. + * + * Use @ref ui_set_statusbar() to display text on the statusbar. + * + * @param format @c printf()-style format string. + * @param ...Arguments for the @c format string. + * + * @see msgwin_status_add_string() + * + * @since 0.15 Seems to be 0.12 actually :] Or drop it, because it wasn't in the original and nobody actually cares > * - * @since 0.15 + * @param msg_color A color to be used for the text. It must be an element of #MsgColors. + * @param line The document's line where the message belongs to. Set to @c -1 to ignore. + * @param doc @nullable The document. Set to @c NULL to ignore. + * @param format@c printf()-style format string. + * @param ... Arguments for the @c format string. + * + * @see msgwin_msg_add_string() + * + * @since 0.16 good catch for 0.16 here -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#pullrequestreview-102837822
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
@b4n ping -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-370045575
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
kugel- commented on this pull request. > * - * @param msg_color A color to be used for the text. It must be an element of #MsgColors. - * @param format @c printf()-style format string. - * @param ... Arguments for the @c format string. + * @see msgwin_compiler_add_string() + * + * @since 0.15 I know what happened. I was looking at c151beff which had added these function under a different name (_add_fmt()). For 0.16.0 they were renamed to the current name. So yes, 0.16 is more accurate. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#discussion_r164145849
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
@kugel- pushed 1 commit. e032739 fixup! msgwin: improve doxygen comments -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1748/files/7541c969abc81dc650c6782af75fcbb3c7658816..e032739afaca7f58f9a6563d743474f2f74499f0
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
b4n requested changes on this pull request. Looks mostly OK, but I'd like the few small fixes > @@ -488,6 +508,30 @@ void msgwin_status_add(const gchar *format, ...) } } +/** + * Logs a status message *without* setting the status bar. + * + * Use @ref ui_set_statusbar() to display text on the statusbar. + * + * @param format @c printf()-style format string. + * @param ...Arguments for the @c format string. + **/ +GEANY_API_SYMBOL +void msgwin_status_add(const gchar *format, ...) +{ + GtkTreeIter iter; unused variable > @@ -488,6 +508,30 @@ void msgwin_status_add(const gchar *format, ...) } } +/** + * Logs a status message *without* setting the status bar. + * + * Use @ref ui_set_statusbar() to display text on the statusbar. + * + * @param format @c printf()-style format string. + * @param ...Arguments for the @c format string. + **/ +GEANY_API_SYMBOL +void msgwin_status_add(const gchar *format, ...) +{ + GtkTreeIter iter; + gchar *string; + gchar *statusmsg, *time_str; unused variables > @@ -410,7 +423,19 @@ void msgwin_msg_add(gint msg_color, gint line, > GeanyDocument *doc, const gchar * } -/* adds string to the msg treeview */ +/** + * Adds a new message in the messages tab treeview in the messages window. + * + * If @a line and @a doc are set, clicking on this line jumps into the + * file which is specified by @a doc into the line specified with @a line. + * + * @param msg_color A color to be used for the text. It must be an element of #MsgColors. + * @param line The document's line where the message belongs to. Set to @c -1 to ignore. + * @param doc @nullable The document. Set to @c NULL to ignore. + * @param stringMessage to be added. + * + * @since @todo + **/ void msgwin_msg_add_string(gint msg_color, gint line, GeanyDocument *doc, const gchar *string) Needs a `GEANY_API_SYMBOL` marker, doesn't it? > void msgwin_compiler_add(gint msg_color, const gchar *format, ...) G_GNUC_PRINTF (2, 3); +void msgwin_compiler_add_string(gint msg_color, const gchar *msg); you should remove the duplicated prototype from the `GEANY_PRIVATE` section below > void msgwin_msg_add(gint msg_color, gint line, GeanyDocument *doc, const gchar *format, ...) G_GNUC_PRINTF (4, 5); +void msgwin_msg_add_string(gint msg_color, gint line, GeanyDocument *doc, const char *msg); Ditto for that one that already existed > * - * @param msg_color A color to be used for the text. It must be an element of #MsgColors. - * @param format @c printf()-style format string. - * @param ... Arguments for the @c format string. + * @param msg_color A color to be used for the text. It must be an element of #MsgColors. + * @param format@c printf()-style format string. + * @param ... Arguments for the @c format string. + * Could be interesting to add a `@see msgwin_compiler_add_string()`, wouldn't it? And similar for the other functions. Just a suggestion though, but could avoid a little confusion for the C API users. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#pullrequestreview-91299682
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
I tested using peasy's python console. I typed Geany.msgwin_status_add_string("xx") and xx appeared in the status tab. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-359957682
Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
LGBI, and the @since needs to be done before commit (Note so it possibly isn't forgotten :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748#issuecomment-359956204
[Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)
This adds _string variants to 3 msgwin_{compiler,status,msg}_add(), since those cannot be gobject-introspected (functions with variadic arguments generally can't because the parameters couldn't be marshalled). So I simply add _string variants which take the string after g_strdup_vprintf(). I found that two of them already existed, so I only needed to add one and export all 3 to plugins. @sagarchalise wants this in peasy and the functions are currently not available in any form. GEANY_API_VERSION increment is TODO, will do once this lands. You can view, comment on, or merge this pull request online at: https://github.com/geany/geany/pull/1748 -- Commit Summary -- * api: add non-variadic variants of msgwin_*_add to the API * msgwin: beautify doxygen comments a bit -- File Changes -- M src/msgwindow.c (117) M src/msgwindow.h (3) -- Patch Links -- https://github.com/geany/geany/pull/1748.patch https://github.com/geany/geany/pull/1748.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1748