On (25/07/14 15:15), Michal Židek wrote:
> On 07/25/2014 01:42 PM, Lukas Slebodnik wrote:
>> On (24/07/14 17:53), Michal Židek wrote:
>>> On 07/24/2014 11:28 AM, Lukas Slebodnik wrote:
>>>> ehlo,
>>>>
>>>> This patch add possibility to replace whitespace in user and group names
with
>>>> a specified string. With string "-", sssd will return the same
result as
>>>> winbind enabled option "winbind normalize names"
>>>>
>>>> Resolves:
>>>>
https://fedorahosted.org/sssd/ticket/1854
>>>>
>>>> patch is attached.
>>>>
>>>> LS
>>>>
>>>
>>> Hi Lukas,
>>>
>>> src/config/SSSDConfig/__init__.py.in
>>> 76 'whitespaces_replacement_string': _('All white spaces will
be replaced
>>> in group name with this string'),
>>>
>>> Not just group name, but also user name. IMO something
>>> like this would be better:
>>> "All white spaces in group or user names will be replaced with
>>> this string." Or something similar.
>>>
>>> Also please put changes in
>>> src/config/SSSDConfig/__init__.py.in and man page changes into a
>>> separate patch. We will than ping a native speaker (Stephen ?)
>>> to check the wording when the coding part is accepted.
>>>
>>> The option name could be at least a little shorter, maybe:
>>> replace_whitespace_str
>>> or
>>> replace_spaces_by
>>>
>>> But this is not a strong requirement, if you do not like the above
>>> names and can't come up with something better, we can stick to the
>>> current name.
>>>
>>> 35 static const char *replace_whitespaces(TALLOC_CTX *mem_ctx,
>>> ... <snip>
>>> 76 while (*ptr != '\0') {
>>> 77 if (isspace(*ptr)){
>>> 78 new_name = talloc_asprintf_append(...
>>> 79 } else{
>>> ^^^^^
>>> Missing space after else
>>> 80 new_name = talloc_asprintf_append(...
>>> 81 }
>>>
>>> The patch seem to work fine. But functions like
>>> replace_whitespaces and the reverse version can be
>>> easily unit-tested. It would be great if you could
>>> add unit tests for these two.
>>>
>>
>> Attached patches fix also comments from other mails in this thread.
>>
>> LS
>>
>
> Thanks for adding the unit tests!
>
> The patches look good to me and passed my testing. So ACK for
> the code part. I would like to have a native English speaker
> to review the man page changes in src/man/sssd.conf.5.xml
>
> Michal
Attached are patches with different text in man page.
Thank you Dan.
LS