Re: Changelog table crashes if it tries to render a commit with an empty message
We landed the fix as
https://kallithea-scm.org/repos/kallithea/changeset/f950740985f47ea09ef3361fa56b3d7581429156
Thanks,
Mads
On 12/15/20 4:10 PM, Brett Smith wrote:
On 12/14/20 5:15 PM, Mads Kiilerich wrote:
Ouch, yeah, .splitlines() is confusingly different from .split('\n')
. Thanks for the patch.
Would it perhaps be better to just use .split instead of .splitlines()?
Your call but on balance I do think splitlines is better. It splits on
all kinds of Unicode line separators
, not
just \n, and I think that's good when you're dealing with very
arbitrary user text like this.
But since the commit message text is shown as a link, would it
perhaps be even better to avoid the void and show a text like "(No
commit message)"?
I think that would be nice. It wasn't in my patch because I wasn't
sure what all was involved with adding a new message string to the
code, but if you can make it happen I think it would be worth it.
In general I didn't think my patch was thorough enough to apply
directly to the code. I sent it just to communicate exactly where I
found the bug.
Thanks,
--
Brett Smith
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Changelog table crashes if it tries to render a commit with an empty message
On 12/14/20 5:15 PM, Mads Kiilerich wrote:
Ouch, yeah, .splitlines() is confusingly different from .split('\n') .
Thanks for the patch.
Would it perhaps be better to just use .split instead of .splitlines()?
Your call but on balance I do think splitlines is better. It splits on
all kinds of Unicode line separators
, not
just \n, and I think that's good when you're dealing with very arbitrary
user text like this.
But since the commit message text is shown as a link, would it perhaps
be even better to avoid the void and show a text like "(No commit
message)"?
I think that would be nice. It wasn't in my patch because I wasn't sure
what all was involved with adding a new message string to the code, but
if you can make it happen I think it would be worth it.
In general I didn't think my patch was thorough enough to apply directly
to the code. I sent it just to communicate exactly where I found the bug.
Thanks,
--
Brett Smith
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Changelog table crashes if it tries to render a commit with an empty message
Ouch, yeah, .splitlines() is confusingly different from .split('\n') .
Thanks for the patch.
Would it perhaps be better to just use .split instead of .splitlines()?
But since the commit message text is shown as a link, would it perhaps
be even better to avoid the void and show a text like "(No commit message)"?
And if it is possible to have an empty commit message, should we perhaps
also have nice handling of the case where the first line is empty?
So something like
--- kallithea/templates/changelog/changelog_table.html
+++ kallithea/templates/changelog/changelog_table.html
@@ -55,7 +55,7 @@
title="${h.fmt_date(cs.date)}">${h.age(cs.date,True)}
- <% message_lines = cs.message.splitlines() %>
+ <% message_lines = cs.message.strip().splitlines() or [_("(No
commit message)")] %>
%if len(message_lines) > 1:
Also, I can see we also have the same problem in pullrequest_show.html ...
/Mads
On 12/14/20 10:04 PM, Brett Smith wrote:
Hi,
I recently upgraded a Kallithea server to 0.6.3, and I was running
into an issue where trying to view the landing page for one specific
repository, but not others, was crashing.
I tracked this down to the changelog_table.html template. It runs
message_lines = cs.message.splitlines() and tries to show the first
line of the message by rendering message_lines[0]. However, if the
commit message is completely empty, message_lines will be too, so
trying to access message_lines[0] raises an uncaught IndexError.
Attached is a patch that does the bare minimum to avoid the crash, by
forcing message_lines to contain an empty string in this case. It
might be nice to do something nicer like render a dedicated, localized
message in this case. But hopefully this helps identify the issue.
Hope this helps. If you need any additional information, please let me
know.
Thanks,
--
Brett Smith
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Changelog table crashes if it tries to render a commit with an empty message
Hi,
I recently upgraded a Kallithea server to 0.6.3, and I was running into
an issue where trying to view the landing page for one specific
repository, but not others, was crashing.
I tracked this down to the changelog_table.html template. It runs
message_lines = cs.message.splitlines() and tries to show the first line
of the message by rendering message_lines[0]. However, if the commit
message is completely empty, message_lines will be too, so trying to
access message_lines[0] raises an uncaught IndexError.
Attached is a patch that does the bare minimum to avoid the crash, by
forcing message_lines to contain an empty string in this case. It might
be nice to do something nicer like render a dedicated, localized message
in this case. But hopefully this helps identify the issue.
Hope this helps. If you need any additional information, please let me know.
Thanks,
--
Brett Smith
diff -r 7b7afdbe57af kallithea/templates/changelog/changelog_table.html
--- a/kallithea/templates/changelog/changelog_table.html Thu Dec 03 01:13:44 2020 +0100
+++ b/kallithea/templates/changelog/changelog_table.html Mon Dec 14 15:58:03 2020 -0500
@@ -55,7 +55,7 @@
${h.age(cs.date,True)}
-<% message_lines = cs.message.splitlines() %>
+<% message_lines = cs.message.splitlines() or [''] %>
%if len(message_lines) > 1:
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
