I haven't read through the code yet, but you should keep to the same naming conventions for your JavaScript files. markdown*.js, not markdown/*.js.

My latest change (up for review) for image review UI has registration support. You should be able to look into that and use it. I recommend applying it to a branch for now, then rebasing your code on top of it, until we get it in (soon).

Place these before the screenshot ones.

Same.

The new branch I mentioned has a FileAttachmentReviewable, which you'll want to look at. It should provide everything this currently does.

Remove the trailing comma.

No need for this.

Make sure not to have trailing commas, or things will break in some browsers.

Trailing commas bad.

Instead of replacing new line characters over here, I can also do {{ review_ui.render|linebreaksbr }} (https://docs.djangoproject.com/en/dev/ref/templates/builtins/?from=olddocs#linebreaksbr)

Is one method better than/preferred over another?

Is there a more elegant / more-Pythonic way to get the FileAttachment object of the Markdown file? 

I don't think this will work if there is a review request with multiple attachments or when the first attachment is not a markdown file

Pretty cool to see markdown being rendered in RB! I couldn't find where to leave comments on the screenshot, but maybe adding a light border and some extra margin (well...padding) around the rendering would help make it clear that we're looking at the file and not some other content, sort of like a page-within-a-page type deal? Just a thought.

I know this is temporary, but just curious what your plans are for handling markdown files. I don't think (?) the mimetype is to be trusted (it might just be text/plain), so will you be augmenting this method to look at file extensions as well?

I might be very wrong here, but I think you need to be inheriting from FileAttachmentReviewUI, otherwise your MarkdownReviewUI won't get picked as the best handler. You'll still need to fix FileAttachmentReviewUI.get_best_handler to support markdown, of course.

See the FileAttachment.review_ui property in attachments/models.py

I might (again) be very wrong here, but your MarkdownReviewUI instance should have been given the FileAttachment object to render when it got instantiated (as `self.obj`). So, instead of getting the file attachments for the review request and picking one from the list, the file to be rendered should already be available as `self.obj.file`. 

See FileAttachmentReviewUI in ui/base.py.

I think the template filter is the better option -- might as well keep that part of the presentation separate.
. It cleans up the python code a little and leaves the door open to getting the regular markdown output if we want.

Out of curiosity, why remove the newlines? Does it affect the rendered HTML?

Sync master and you'll be able to take advantage of registration. Look at reviewboard/reviews/ui/__init__.py.

This is a bit expensive.

You should use the file attachment provided. You'll need to subclass FileAttachmentReviewUI for that.

I don't think you need to do this. It's more expensive to process this than to just let the browser handle it.

Also, if we were going to, a regex would be faster. One loop through and not 3.

Unless you have additional work to do here, don't bother overriding it.

You shouldn't define a new type of render function. What you really should be doing is:

    this.$el.html(this.model.get('rendered'));

in renderContent.

No need to return anything.

You want {{markdown.caption|escapejs}}

Same.

This should append view.$el.

Should be included alphabetically, and have a trailing comma.

There is no standardized mimetype for Markdown yet. I tested with a few Markdown files and were plain text, some were "text/x-markdown" and some were "application/octet-stream".

I think we should do as follows:
- support_mimetypes should be ['text/x-markdown', 'text/x-web-markdown'] (until something is standardized)
- Additionally, there should be a property to match against the file extension. For example: extension_pattern = "*.md"

Instead of relying on mimetypes, we can use the file extension to match an attachment to a pluggable UI.

It would be nice to simply prioritize the mimetype when looking for a best handler. So, instead of checking the file extension here, let get_best_handler run its course. If it couldn't find a handler using the mimetype, then check the file extension.  This way, if, say a jpeg is uploaded with the right mimetype, but a file extension in the MIMETYPE_EXTENSIONS, it will still pick the right handler.

Also, checking the extension separately should make it a little easier to have many file extensions for the same file. You could add a class property of supported_file_extensions, and fill it with a list of file extensions. I know, for me, I've used '.md', '.mdown' and '.markdown' for Markdown files.

Any thoughts?

This would be more visible at the top of the file.

Other than the comments I've left, this is looking pretty good to me.

I wonder if it'd make more sense to just call it reviewboard.reviews.ui.markdown, as opposed to reviewboard.reviews.ui.markdownui. The ui in markdownui is a bit redundant.

Just a nit.

Maybe I'm missing something here, but why are you changing these? Seems only tangentially related to the rest of the patch.

We tend to keep the trailing commas in Python.

Hey - so the code looks pretty good, and I tried the patch and was able to render some Markdown, which was awesome.

I'm curious though - am I supposed to be able to attach comments to this Markdown? Did I miss something? Where does that come in?

Again, I have to ask - why are these being added? I don't see these files being added in this patch...

What's needed before this can go in? Anything, or is it standalone at this point?

Should inherit from FileAttachmentReviewable, and then you won't need 'caption' or 'attachmentID'.

Nit: alphabetical

Nit: remove this newline.

I think this is indented too much.

StringIO is part of the standard library, so your imports should read like this:

import StringIO

import markdown

from reviewboard.reviews.ui.base import FileAttachmentReviewUI

It looks like this is including the text change as well. Can you make sure you're using --parent?

Should be:

try:
    from cStringIO import StringIO
except ImportError:
    from StringIO import StringIO


That'll use cStringIO when available, which is faster.