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
LGBI, and the @since needs to be done before commit (Note so it possibly isn't forgotten :)
I tested using peasy's python console. I typed Geany.msgwin_status_add_string("xx") and xx appeared in the status tab.
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 string Message 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.
@kugel- pushed 2 commits.
ad62faf msgwin: improve doxygen comments 7541c96 fixup! api: add non-variadic variants of msgwin_*_add to the API
b4n requested changes on this pull request.
Otherwise looks good
- * @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
Is that correct? Looks to me it's new in 921f010ffc38aaadf7118161b2f85df58e7bfc5c which seems to be [0.16](https://github.com/geany/geany/blob/921f010ffc38aaadf7118161b2f85df58e7bfc5c...) It doesn't matter much at all, but wrong info isn't nice.
@@ -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
Same
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
Yes, you are right. I didn't spot 0.15 when going backwards in the log starting from that commit. Next time I shall just ask `git describe`
@kugel- pushed 1 commit.
e032739 fixup! msgwin: improve doxygen comments
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.
@b4n ping
@b4n ping
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
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?
@b4n I would think you could just write "Closes #XXX" in the final commit message, is that what you're looking for?
I'll amend according to your comments and give you a chance to review again. But in case you don't get another chance for 1.34 I'd go ahead and merge prior to the freeze since you indicated that the rest looks good to you. Is that approach OK for you?
@kugel- pushed 1 commit.
daa9800 fixup! msgwin: improve doxygen comments
@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.
Closed #1748 via 1611e3f94960a821ced0af9b99052afa6ade2111.
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*`).
@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.
@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.
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.
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.
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<<name<<" is "<<age; ```
Oh, in C!! :grin:
To be fair to GI (what! me saying that!) most languages do not have variadic functions, so the GI has nothing to translate into. For example what Python (even Cython) would it be specified as?[0]
So it would be a waste of time for GI to parse it anyway since nothing but C and C++ can use it and they can call the C API directly.
[0] actually Python ctypes can call variadic functions as completely untyped functions, but the arguments can't be specified, so GI can't be used, [see](https://docs.python.org/3/library/ctypes.html#specifying-the-required-argume...)
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); } ```
Ok, so I forgot to specify "C style variadic functions", being able to call their own functions with variable arguments isn't the same as calling a C function that way.
They don't need to call C directly, GI just needs to generate the appropriate adapters in the typelib.
github-comments@lists.geany.org