Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(8)

Issue 7220043: Moin IncludeWithVals plugins

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 5 months ago by gordon.messmer
Modified:
2 years, 4 months ago
Reviewers:
ReimarBauer
Visibility:
Public.

Description

Moin IncludeWithVals plugins

Patch Set 1 #

Total comments: 16
Unified diffs Side-by-side diffs Delta from patch set Stats (+850 lines, -0 lines) Patch
M COPYING View 1 chunk +674 lines, -0 lines 1 comment Download
M README View 1 chunk +29 lines, -0 lines 0 comments Download
M action/savedict.py View 1 chunk +79 lines, -0 lines 10 comments Download
M macro/IncludeVal.py View 1 chunk +31 lines, -0 lines 2 comments Download
M macro/IncludeWithVals.py View 1 chunk +37 lines, -0 lines 3 comments Download

Messages

Total messages: 2
ReimarBauer
some comments, first pass through https://codereview.appspot.com/7220043/diff/1/COPYING File COPYING (right): https://codereview.appspot.com/7220043/diff/1/COPYING#newcode1 COPYING:1: GNU GENERAL PUBLIC LICENSE ...
2 years, 5 months ago (2013-01-26 22:13:56 UTC) #1
gordon.messmer
2 years, 4 months ago (2013-03-03 16:49:19 UTC) #2
Apparently I left these comments in Draft status, and did not send them. :(

https://codereview.appspot.com/7220043/diff/1/action/savedict.py
File action/savedict.py (right):

https://codereview.appspot.com/7220043/diff/1/action/savedict.py#newcode26
action/savedict.py:26: page.send_page(send_special=True)
I don't know what it does, specifically.  That code came directly from
action/edit.py:execute

https://codereview.appspot.com/7220043/diff/1/action/savedict.py#newcode34
action/savedict.py:34: raise ValueError("You don't have enough rights on this
page")
This code matches the same test in macro/__init__.py:macro_GetVal

https://codereview.appspot.com/7220043/diff/1/action/savedict.py#newcode42
action/savedict.py:42: savedict[k] = request.form.get(k)
The values are escaped on display.  I'm pretty sure that escaping them here
would double-escape them.

https://codereview.appspot.com/7220043/diff/1/action/savedict.py#newcode46
action/savedict.py:46: savetext = savetext + u' ' + k + u':: ' + savedict[k] +
u'\n'
Concatenation is faster in cpython.  According to PEP8, though, I should
probably convert this to ''.join()

https://codereview.appspot.com/7220043/diff/1/action/savedict.py#newcode67
action/savedict.py:67: request.theme.add_msg(unicode(msg), "error")
Again, I'm not sure.  This code is also identical to the same function in
action/edit.py:execute

https://codereview.appspot.com/7220043/diff/1/macro/IncludeVal.py
File macro/IncludeVal.py (right):

https://codereview.appspot.com/7220043/diff/1/macro/IncludeVal.py#newcode13
macro/IncludeVal.py:13: _sysmsg = '<p><strong class="%s">%s</strong></p>'
Sounds right.  This code matches macro/Include.py, where sysmsg is also
specified as a non-unicode string.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld e6eadc5