Clément Lassieur <clement@lassieur.org> writes:

> Christopher Baines <mail@cbaines.net> writes:
>
>> * gnu/services/cgit.scm (serialize-cgit-configuration): Add the extra options,
>> one per line, before the scan-path, as this makes it possible to use the
>> extra-options to affect the global behaviour for repositories.
>> ---
>>  gnu/services/cgit.scm | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/gnu/services/cgit.scm b/gnu/services/cgit.scm
>> index 8ef12cd5a..bacd6ca16 100644
>> --- a/gnu/services/cgit.scm
>> +++ b/gnu/services/cgit.scm
>> @@ -642,13 +642,19 @@ for cgit to allow access to that repository.")
>>      (not (memq (configuration-field-name field)
>>                 '(project-list
>>                   repository-directory
>> -                 repositories))))
>> +                 repositories
>> +                 extra-options))))
>
>                       ---^
> Very small nitpick: could you put extra-options between project-list and
> repository-directory there?  So that the order is the same with what
> comes below.

Sure, this should be fixed now.

>>    #~(string-append
>>       #$(let ((rest (filter rest? cgit-configuration-fields)))
>>           (serialize-configuration config rest))
>>       #$(serialize-project-list
>>          'project-list
>>          (cgit-configuration-project-list config))
>> +     #$(string-join
>> +        (append
>> +         (cgit-configuration-extra-options config)
>> +         '(""))
>> +        "\n")
>
> Could you put the serialization code in a dedicated function?
> (e.g. SERIALIZE-EXTRA-OPTION)  So that SERIALIZE-CGIT-CONFIGURATION is
> only responsible for the field ordering.

Done.

> Also, I don't understand why you append '("").  If it's just to make
> cgitrc look prettier, I'm not sure it's a good idea.  But maybe I missed
> something?

The '("") was to add an extra newline effectively. I've replaced this by
using the 'suffix option of string-join in the updated patch.