Code factorization

[Tamaranch]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, July 5, 2020 6:05 PM, Matthew Brush <mbrush at codebrainz.ca> wrote:

> On 7/4/2020 2:30 PM, Tamaranch wrote:
>
> > Hi everyone,
> > The following concerns potentially all XFCE components, so I thought of using the mailing list rather than Gitlab: sorry if it wasn't a good idea, and thanks by advance to indicate me a better place for this type of comment in this case.
> > In a previous MR (https://gitlab.xfce.org/xfce/libxfce4util/-/merge_requests/1), I added the function xfce_append_quoted() and the macro STR_IS_EMPTY() to libxfce4util API, in order to replace later their various occurrences in XFCE components, where they appear in one form or another.
> > Now that I took a closer look, I don't know how this should be done, or even if this should be done, despite the fact that using this kind of utilities from only one (or a few) central place, seems obviously a better way to do than duplicating the code in all components that need them.
> > Here are the two problems I encounter:
> >
> > 1.  Using these utilities from a central place implies to add this central place as a dependency to many XFCE components, which is not necessarily harmless.
> > 2.  Should there be one central place or several, and if there is only one, which one?
> >     For instance, in the case of STR_IS_EMPTY(), exo appears as a concurrent to libxfce4util, with its exo_str_is_empty().
> >
> >
> > To finish and illustrate, here is a list of XFCE components where I found occurrences of these patterns:
> >
> > 1.  _append_quoted:
> >     xfce4-panel: https://gitlab.xfce.org/search?utf8=✓&search=_append_quoted&group_id=&project_id=13&search_code=true&repository_ref=master&nav_source=navbar
> >     tumbler: https://gitlab.xfce.org/search?utf8=✓&search=_append_quoted&group_id=&project_id=10&search_code=true&repository_ref=master&nav_source=navbar
> >     thunar: https://gitlab.xfce.org/search?utf8=✓&search=_append_quoted&group_id=&project_id=8&search_code=true&repository_ref=master&nav_source=navbar
> >
> > 2.  str_is_empty:
> >     xfce4-panel: https://gitlab.xfce.org/search?utf8=✓&snippets=&scope=&repository_ref=master&search=str_is_empty&project_id=13
> >     thunar: https://gitlab.xfce.org/search?utf8=✓&snippets=&scope=&repository_ref=master&search=str_is_empty&project_id=8
> >     xfce4-settings: https://gitlab.xfce.org/search?utf8=✓&search=str_is_empty&group_id=&project_id=16&search_code=true&repository_ref=master&nav_source=navbar
> >     exo: https://gitlab.xfce.org/search?utf8=✓&search=str_is_empty&group_id=&project_id=4&search_code=true&repository_ref=master&nav_source=navbar
> >     xfce4-whiskermenu-plugin: https://gitlab.xfce.org/search?utf8=✓&search=str_is_empty&group_id=&project_id=74&search_code=true&repository_ref=master&nav_source=navbar
> >     xfce4-mailwatch-plugin: https://gitlab.xfce.org/search?utf8=✓&search=str_is_empty&group_id=&project_id=55&search_code=true&repository_ref=master&nav_source=navbar
> >     xfce4-indicator-plugin: https://gitlab.xfce.org/search?utf8=✓&search=str_is_empty&group_id=&project_id=53&search_code=true&repository_ref=master&nav_source=navbar
> >
> >
> > Note that it remains the components where the code of STR_IS_EMPTY() (or ! STR_IS_EMPTY()) appears directly in one form or another, like in mousepad (I did not search for all these occurrences yet):
> > text == NULL || *text == '\0'
> > filename != NULL && *filename != '\0'
>
> Hi Tamaranch,
>
> Is it really worth hiding simple C code like `if (a && *a)` (or the
> inverse/variants thereof) behind a macro?
>
> If it is, I would suggest to at least prefix the name with a "namespace"
> to prevent clashes. Better yet, use an inline static function with a
> prefix, that will not evaluate the argument twice and will likely result
> in the same compiled code.
>
> As to where to put such code, in my opinion, non-Xfce-specific general
> purpose utility code would best be upstreamed into Glib since that's
> what its purpose is. If it can't/won't be merged upstream, then
> xfce4-util seems like a fine place, and looking at its dependencies, it
> doesn't seem like too big of a deal for other apps/components to depend
> on it.
>
> Regards,
> Matthew Brush
>
> Xfce4-dev mailing list
> Xfce4-dev at xfce.org
> https://mail.xfce.org/mailman/listinfo/xfce4-dev

Thanks! :)
This confirms to me that I should rather focus on xfce_expand_field_codes() and xfce_append_quoted() first.

Regarding glib, I didn't find any trace of a request for such a macro on gitlab, but there is this "old" discussion (from 2007 to 2013) about it: https://bugzilla.gnome.org/show_bug.cgi?id=399880
Interesting (even if I didn't read everything), but I feel like it is useless to add a new comment to this discussion, by opening a new issue on gitlab: the maintainers definitely don't want to add this to glib.

Best regards,
Tamaranch