Re: [Github-comments] [geany/geany] GI compatible msgwin_*_add (#1748)

[Matthew Brush]

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)

[Matthew Brush]

> 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)

[elextr]

> > 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)

[Matthew Brush]

> 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)

[Thomas Martitz]

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)

[Matthew Brush]

@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)

[Colomban Wendling]

@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)

[Matthew Brush]

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)

[Colomban Wendling]

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)

[Colomban Wendling]

@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)

[Thomas Martitz]

@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)

[Colomban Wendling]

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)

[Colomban Wendling]

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)

[Thomas Martitz]

@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)

[Thomas Martitz]

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)

[Thomas Martitz]

@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)

[Colomban Wendling]

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)

[Thomas Martitz]

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)

[elextr]

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)

[Thomas Martitz]

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