Code factorization

[Tamaranch]

Ok, thanks for these indications :)

I continued to search for the pattern "= '\0'", and I found many more components where STR_IS_EMPTY() could be used.
But this also led me to find places where xfce_expand_field_codes() could be used (the main function I added in https://gitlab.xfce.org/xfce/libxfce4util/-/merge_requests/1), so I also searched systematically for the pattern "= '%'" to find all of them.

Now I think I will start with these cases (at least when libxfce4util is already a dependency), in a component by component approach, taking the opportunity to also replace the occurrences of STR_IS_EMPTY() and xfce_append_quoted(), and see what is the opinion of the maintainers of each project about these changes.

Best regards,
Tamaranch

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Saturday, July 4, 2020 11:38 PM, Simon Steinbeiss <simon at xfce.org> wrote:

> Hi Tamaranch,
>
> first of all thanks a lot for making the effort to cleaning up code and making it more maintainable! Most people are only interested in adding (or requesting) features.
> Secondly the mailing list is a good - albeit sometimes rather quiet - place for such general discussions.
>
> ad 1) I would say that for most core components the dependency on libxfce4util is rather harmless (xfce4-panel, thunar, xfce4-settings already depend on it). And by extension panel plugins can decide to rely on libxfce4util because this library is anyway required by the panel...
> For components that may also be used independently (like tumbler, iirc it was used for Nokia's own Android competitor back in the day) this can be a separate discussion if it's really just about a single macro vs. the dependency on an additional library to cover that. I would
>
> ad 2) The central place for the two functions you started to unify was also well-chosen as I'd rather take things out of exo than continue to build it up. From where I stand there are a few things in exo which should perform an exodus (pun intended) - much as we did for the exo-helpers and preferred apps dialog already by moving it to xfce4-settings.
>
> But these are just my personal 2cents.
> Cheers
> Simon
>
> On Sat, Jul 4, 2020 at 11:36 PM Tamaranch <trash.paradise at protonmail.com> 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'
>>
>> Best regards,
>> Tamaranch
>>
>> _______________________________________________
>> Xfce4-dev mailing list
>> Xfce4-dev at xfce.org
>> https://mail.xfce.org/mailman/listinfo/xfce4-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.xfce.org/pipermail/xfce4-dev/attachments/20200705/a900a769/attachment.html>