[PATCH v2] comments: avoid confusing 'No comments' on empty general comments

[Mads Kiilerich]

On 04/25/2015 08:34 PM, Thomas De Schampheleire wrote:
> # HG changeset patch
> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
> # Date 1429818259 -7200
> #      Thu Apr 23 21:44:19 2015 +0200
> # Node ID 471b0adcad4cb7e40db3599340fe1b6242d3a7b8
> # Parent  accdfd58edfb0687cfb2e59e619e2f90ed8f0d35
> comments: avoid confusing 'No comments' on empty general comments

Thanks - I pushed most of this change; all the refactorings except the 
actual text change ...

> diff --git a/kallithea/controllers/pullrequests.py b/kallithea/controllers/pullrequests.py
> --- a/kallithea/controllers/pullrequests.py
> +++ b/kallithea/controllers/pullrequests.py
> @@ -696,7 +696,7 @@
>           if allowed_to_change_status:
>               status = request.POST.get('changeset_status')
>               close_pr = request.POST.get('save_close')
> -        text = request.POST.get('text', '').strip() or _('No comments.')
> +        text = request.POST.get('text', '').strip()
>           if close_pr:
>               text = _('Closing.') + '\n' + text

This Close text should really also be added in the template instead ...

> diff --git a/kallithea/templates/changeset/changeset_file_comment.html b/kallithea/templates/changeset/changeset_file_comment.html
> --- a/kallithea/templates/changeset/changeset_file_comment.html
> +++ b/kallithea/templates/changeset/changeset_file_comment.html
> @@ -51,7 +51,13 @@
>         %endif
>         </div>
>         <div class="text">
> +        %if not co.text:
> +          <div class="rst-block automatic-comment">
> +            <p>[ Automatic comment: status change ]</p>

It is already css styled and italic so no need for "ascii art".  It is 
also no longer an "automatic" comment message instead of one entered by 
the user; it is just information from the system. The text should also 
be localized.

It is hard to come up with a really good text for explaining this. I 
wonder why ...

But thinking about it ... what is really the problem here? Status 
changes looked "wrong" when the "div where the users comments is shown" 
was wrong. We thus try to find a text to explain the situation and work 
around it. So I guess the real problem is that the status change marker 
is shown in the header where it not is obvious that the status change 
_was_ the comment from the user. What if we moved the status change 
bullet+text inside the text div after the comment (without showing the 
comment if it is empty)?

> +          </div>
> +        %else:
>             ${h.rst_w_mentions(co.text)|n}
> +        %endif
>         </div>
>       </div>
>     </div>

/Mads