|
|
|
Created:
17 years, 2 months ago by georgedorn Modified:
16 years, 5 months ago Base URL:
http://rietveld.googlecode.com/svn/trunk/ Visibility:
Public. |
Patch Set 1 #
Total messages: 3
|
georgedorn
There. This seems to work and was easier than expected.
|
17 years, 2 months ago (2008年10月29日 18:05:23 UTC) #1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There. This seems to work and was easier than expected.
http://codereview.appspot.com/7888/diff/1/6 File templates/diff_navigation.html (right): http://codereview.appspot.com/7888/diff/1/6#newcode10 Line 10: {{patch.prev.key.id}}{%if context and column_width%}?context={{context}}&column_width={{column_width}} There should be a better way to do this.
I had some troubles applying the patch, because it tries to patch a non existent templates/view_details_select.html. But in general it works great, we just have some (irrelevant) layout issues with column headers and comments when column width is < 80. I think that isn't a problem at all... ;-) Some lines in the patch are > 79 chars, I didn't commented them. http://codereview.appspot.com/7888/diff/1/4 File codereview/engine.py (right): http://codereview.appspot.com/7888/diff/1/4#newcode186 Line 186: colwidth=DEFAULT_COLUMN_WIDTH, debug=False, context=DEFAULT_CONTEXT): Line too long and missing blank line above "def RenderDiffTableRows..." http://codereview.appspot.com/7888/diff/1/2 File codereview/views.py (right): http://codereview.appspot.com/7888/diff/1/2#newcode289 Line 289: column_width = forms.IntegerField(label='Column Width', initial=80) Maybe you should add max_value and min_value here (see http://docs.djangoproject.com/en/dev/ref/forms/fields/#integerfield). Without it it's no problem to set a negative value or some verrry high value (raises a BadValue error) here (of course, this is catched in _get_column_width_for_user()). http://codereview.appspot.com/7888/diff/1/5 File templates/view_details_select.html (right): http://codereview.appspot.com/7888/diff/1/5#newcode11 Line 11: <label for="id_column_width">Column Width:</label> hm... this makes the box on the top very large... The more we add to this box the more I feel unlucky with it. I think we should find a better place for those options and links some day, but for now it's ok to leave them where they are.