On 01/02/2016 01:15 PM, Jan Heylen wrote:
Hi,
third version of this patchset, with some extra fixes and partly refactoring of
the applicable code (diff_block)
more work to get both templates uniform are WIP.
Thanks. Much better!
As you can see, I took some of your changes and reworked them to my
liking instead of just commenting. I hope that is ok. Some of the more
"controversial" changes have been left out, but they should be easier to
do now and be easier to present cleanly.
General comments:
Please try to
* put one change in each changeset
* make good commit messages that explain why the change is made and what
is achieved
* order them by "risk" and "likely to be taking without more discussion"
- refactorings first, then features
--- a/kallithea/templates/changeset/diff_block.html Sat Jan 02 11:53:38
2016 +0100
+++ b/kallithea/templates/changeset/diff_block.html Sat Jan 02 11:54:11
2016 +0100
@@ -31,6 +31,12 @@
<span target="${'diff-container-%s' % (id(change))}"
class="diff-collapse-button">↑ ${_('Collapse Diff')} ↑</span>
</div>
<div class="diff-container" id="${'diff-container-%s' % (id(change))}">
+ <span style="float:right;margin-bottom:3px;margin-right:30px">
+ <label>
+ ${_('Show inline comments')}
+
${h.checkbox('',checked="checked",class_="show-inline-comments",id_for="diff-container-%s"
% (id(change)))}
+ </label>
+ </span>
%for FID,(cs1, cs2, change, path, diff, stats) in change.iteritems():
<div id="${FID}_target" style="clear:both;margin-top:25px"></div>
<div id="${FID}" class="diffblock margined comm">
id() will essentially just take the memory address of the variable. That
is not good to use here. Other objects might get the same address when
this goes out of scope. Actually, it seems like you in this case only
need a globally unique identifier?
I don't know how much more convincing the commit messages can be made,
but I'm very reluctant to introduce code for hiding the diff after
fetching it. I think it must be lazily loaded when the diff actually is
shown.
I also see more use case for collapsing the individual file diffs (after
they have been reviewed so the reviewer can focus on the rest) than for
collapsing everything (leaving almost nothing on the page, only to guide
the reader somewhere else).
/Mads
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
