Code factorization

[Matthew Brush]

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=%E2%9C%93&search=_append_quoted&group_id=&project_id=13&search_code=true&repository_ref=master&nav_source=navbar
> tumbler: https://gitlab.xfce.org/search?utf8=%E2%9C%93&search=_append_quoted&group_id=&project_id=10&search_code=true&repository_ref=master&nav_source=navbar
> thunar: https://gitlab.xfce.org/search?utf8=%E2%9C%93&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=%E2%9C%93&snippets=&scope=&repository_ref=master&search=str_is_empty&project_id=13
> thunar: https://gitlab.xfce.org/search?utf8=%E2%9C%93&snippets=&scope=&repository_ref=master&search=str_is_empty&project_id=8
> xfce4-settings: https://gitlab.xfce.org/search?utf8=%E2%9C%93&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=%E2%9C%93&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=%E2%9C%93&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=%E2%9C%93&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=%E2%9C%93&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