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