Skip to content


Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP


Wikicleanup3 #528

merged 0 commits into from

3 participants


Third wiki cleanup, needs some testing tomorrow (especially the>pre and modhash stuff), right now I need to sleep (6am). Modhash validation is not in opensource and needs actual testing.

((6 lines not shown))
@wiki_validate(page=VWikiPage('page', restricted=True, modonly=True))
def GET_wiki_settings(self, page):
settings = {'permlevel': page._get('permlevel', 0)}
mayedit = page.get_editors()
return WikiSettings(settings, mayedit, show_settings=not page.special).render()
@wiki_validate(page=VWikiPage('page', restricted=True, modonly=True),\
@kemitche Collaborator

(I'm not doing a full review, just glancing at things over morning coffee)

The trailing slash isn't needed, since you're inside parens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

The current way c.show_wiki_actions is implemented is really ugly -- it's basically a global. There shouldn't need to be a line in the base controller to determine whether or not a UI element displays. Can you please restructure things so that the view code determines whether or not it's appropriate to display the box?


I believe the "use user stylesheet" stuff is in the base controller and a global, I was modeling it off of that.

edit: We landed on this in irc, going to do things still globally but locally globally ;)


@andre-d But this is for a UI box that only shows on specific pages in specific controllers. Stylesheets are something that apply to most if not all subclasses of the base controller.

((19 lines not shown))
+ pages, linear_pages = WikiPage.get_listing(, filter_check=check_hidden)
+ return WikiListing(pages, linear_pages).render()
+ def handle_error(self, code, reason=None, **data):
+ abort(WikiError(code, reason, **data))
+ def pre(self):
+ RedditController.pre(self)
+ if g.disable_wiki and not c.user_is_admin:
+ self.handle_error(403, 'WIKI_DOWN')
+ if not
+ self.handle_error(404, 'NOT_WIKIABLE') # /r/mod for an example
+ frontpage = isinstance(, DefaultSR)
+ base = '' if frontpage else '/r/%s' %

I believe this is equivalent to just

andre-d added a note

Excellent, thanks :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

It ends up being just the same code wise since we still need to validate that the user can create the page either way. The only difference is that the user presses create, then edits the page. Rather than goes to create, edits the page, then presses submit to create the page which then creates and revises the page doing both checks in the same call ONLY if the page does not exist, if the page does exist it only does the latter half of the checks. Thus, more complicated in my mind.


As a general comment: please read over your pull requests more thoroughly before asking for review. There are a lot of little issues like irrelevant things in commits and extra whitespace that you'd catch if you gave your commits a thorough look over. These things all slow reviewers down. Every thing a reviewer has to make a comment about is more work for you and more work for the reviewer. Please catch them early by looking over your commits with a fine-tipped comb.


As a general comment, the informal format I've been using for commits (cribbed from @spladug) is:

lowercasefeaturename: Capitalized sentence with a period at the end.

Not worth changing existing commits over, but my OCD would appreciate if you used it in the future.

@andre-d andre-d merged commit a65a35f into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Sorry, commit information is not available for this pull request.

Something went wrong with that request. Please try again.