Re: [GSoC][PATCH 0/2] rebase -i: rewrite append_todo_help() in C

> On Thu, May 31, 2018 at 10:48 AM, Phillip Wood
> <phillip.wood@talktalk.net> wrote:
>> Hi Alban, it's great to see you working on this
>>
>> On 31/05/18 12:01, Alban Gruin wrote:
>>> This series rewrites append_todo_help() from shell to C. This is part
>>> of the effort to rewrite interactive rebase in C.
>>>
>>> The first commit rewrites append_todo_help() in C (the C version
>>> covers a bit more than the old shell version), adds some parameters to
>>> rebase--helper, etc.
>>
>> I've had a read of the first patch and I think it looks fine, my only
>> comment would be that the help for '--edit-todo' is a bit misleading at
>> the moment as currently it's just a flag to tell rebase-helper that the
>> todo list is being edited rather than actually implementing the
>> functionality to edit the list (but hopefully that will follow in the
>> future).
> 
> Would you have better suggestions for the name of the flag?
> Of the top of my head:
>    --write-edit-todo
>    --hint-todo-edit
>    --include-todo-edit-hint
> not sure I like these names, though they seem to reflect the
> nature of that flag a little bit better.
> 
>>> The second one strips newlines from append_todo_help() messages, which
>>> require to update the translations. This change was advised to me by
>>> Stefan Beller, but Johannes Schindelin voiced concerns. I don’t really
>>> have a strong opinion about it, so feel free to give yours.
>>
>> I'm not sure I understand what the point of this patch is, if the
>> newlines are unnecessary then I'd just omit them from the first patch -
>> am I missing something?
>>
> 
> The new lines are part of the output and are currently in the part to
> be translated:
> For example from the German translation file:
> 
> #: git-rebase--interactive.sh:171
> msgid ""
> "\n"
> "Do not remove any line. Use 'drop' explicitly to remove a commit.\n"
> msgstr ""
> "\n"
> "Keine Zeile entfernen. Benutzen Sie 'drop', um explizit einen Commit zu\n"
> "entfernen.\n"
> 
> After patch 2 is applied, the translators only see
> "Do not remove any line. Use 'drop' explicitly to remove a commit."
> as a need to translate, and the two additional new lines (one in front
> and one after the string) are just put in place autormatically.
> 
> Usually we do not want to play sentence lego, but this is a whole
> sentence for translation; it is rather about formatting the output for
> the terminal, adding new lines to separate some messages.
> 
> I thought this patch would just show goodwill towards translators
> that do not need to replicate the formatting exactly.
> 
> If you feel strongly, I'd rather see Alban drop this second patch and
> move on instead of waiting for our argument to settle. ( I do not feel
> strongly about it, but put it out as a suggestion as that seemed like
> it would lead to a better end state for the project).