Review Request #4555: Make the "Diff Revision" label dynamically rendered in javascript.

Description	From	Last Updated
'from settings_local import *' used; unable to detect undefined names	reviewbot	Sept. 10, 2013, 8:20 p.m.
The <h1> shouldn't be part of the localized string.	chipx86	Sept. 11, 2013, 11:58 a.m.
Same.	chipx86	Sept. 11, 2013, 11:58 a.m.
It doesn't seem that all this should be localized. The "Diff revision XYZ" should, and the "This is not ..." …	chipx86	Sept. 11, 2013, 11:59 a.m.
This should also be two strings, and should not involve tags. That's a layout thing for a template to deal …	chipx86	Sept. 11, 2013, 11:59 a.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Sept. 11, 2013, 12:11 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Sept. 11, 2013, 1:36 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Sept. 11, 2013, 1:37 p.m.
This is the only detail string without tags. Do we want to make sure we escape here?	chipx86	Sept. 11, 2013, 4:43 p.m.
Maybe just have a string for the revision, and build the common parts after (or in the call to window.location). …	chipx86	Sept. 11, 2013, 4:26 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Sept. 11, 2013, 5:23 p.m.

This is a review from Review Bot.
Tool: PEP8 Style Checker
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

This is a review from Review Bot.
Tool: Pyflakes
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
'from settings_local import *' used; unable to detect undefined names
```

reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js (Diff revision 1)
The issue has been resolved. Show all issues
```
The <h1> shouldn't be part of the localized string.
```
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Same.
```

reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js (Diff revision 1)

The issue has been resolved. Show all issues

It doesn't seem that all this should be localized. The "Diff revision XYZ" should, and the "This is not ..." should, and the "See what's changed" should, but not as one big string.

The layout of this is going to change quite a lot soon anyway.

david Sept. 11, 2013, 11:59 a.m.

I'll split the header and paragraph, but I disagree about splitting apart the sentences entirely or completely pulling tags out of the strings. I plan to re-do this all after making a new revision selector, though (it's crummy that it reflows the page).

chipx86 Sept. 11, 2013, 3:52 p.m.

Completely pulling out tags isn't necessarily, but formatting and layout are different. I guess in this case, "See what's changed" is fine being part of the same sentence, but if we decided to change layout, we can affect the strings. No big deal for now though.

Let's talk about the revision selector offline. I've spent some time thinking about it and making mockups, so I want to sync up on that.

reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js (Diff revision 1)
The issue has been resolved. Show all issues
```
This should also be two strings, and should not involve tags. That's a layout thing for a template to deal with.
```

Diff:

Revision 2 (+153 -37)

Show changes

	reviewboard/settings.py
	reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
	reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.js
	reviewboard/templates/diffviewer/view_diff.html

This is a review from Review Bot.
Tool: PEP8 Style Checker
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

This is a review from Review Bot.
Tool: Pyflakes
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
'from settings_local import *' used; unable to detect undefined names
```

Diff:

Revision 3 (+153 -37)

Show changes

	reviewboard/settings.py
	reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
	reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.js
	reviewboard/templates/diffviewer/view_diff.html

This is a review from Review Bot.
Tool: PEP8 Style Checker
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

This is a review from Review Bot.
Tool: Pyflakes
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
'from settings_local import *' used; unable to detect undefined names
```

Change Summary:

Empty out the element before sticking new data in. This isn't important for this change today, but will be once we get rid of reloads.

Diff:

Revision 4 (+153 -37)

Show changes

	reviewboard/settings.py
	reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
	reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.js
	reviewboard/templates/diffviewer/view_diff.html

This is a review from Review Bot.
Tool: PEP8 Style Checker
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

This is a review from Review Bot.
Tool: Pyflakes
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
'from settings_local import *' used; unable to detect undefined names
```

reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js (Diff revision 4)

So here's another issue to figure out involving localization and tags and such.

In the main template, we're using <%= detail %>, so we won't get escaping. This is clearly done because the other templates contain tags, which is fine.

Now, way we localize and get some characters in the strings that need to be escaped. We now have no mechanism to escape them, so the translated strings themselves will have to be escaped. That means we have to trust the strings to never be wrong.

What are your thoughts on that?

david Sept. 11, 2013, 4:45 p.m.

I'm going to add translator comments here to explain what to do.

david Sept. 11, 2013, 4:46 p.m.

Also note that while I haven't thought too much about it, I do want to replace all of this later. For now I wanted to keep the same functionality while enabling reload-free revision switches.

chipx86 Sept. 11, 2013, 10:17 p.m.

Sure, that's fine. It's more of a higher-level discussion I think.

reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js (Diff revision 4)

Any reason we don't just operate on this.$el instead?

(I'm assuming it's for expansion, but just wanted to check.)

reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js (Diff revision 4)
The issue has been dropped. Show all issues
```
This is the only detail string without tags. Do we want to make sure we escape here?
```

reviewboard/static/rb/js/pages/views/diffViewerPageView.js (Diff revision 4)

The issue has been dropped. Show all issues

Maybe just have a string for the revision, and build the common parts after (or in the call to window.location). No sense in duplicating the diff/ and #index_header parts.

david Sept. 11, 2013, 4:45 p.m.

This code is extremely temporary, and will be going away when I eliminate the reload. We'll have something similar for updating the history, but I'm doing to defer improving it until then.

chipx86 Sept. 11, 2013, 10:17 p.m.
```
Sure.
```

Diff:

Revision 5 (+153 -37)

Show changes

	reviewboard/settings.py
	reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
	reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.js
	reviewboard/templates/diffviewer/view_diff.html

This is a review from Review Bot.
Tool: PEP8 Style Checker
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

This is a review from Review Bot.
Tool: Pyflakes
Processed Files:
reviewboard/settings.py
Ignored Files:
reviewboard/templates/diffviewer/view_diff.html
reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.js
reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
reviewboard/static/rb/js/pages/views/diffViewerPageView.js

reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
'from settings_local import *' used; unable to detect undefined names
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (f9f8751).

Review Board + Power Pack 7.1 alpha 0 (dev)

Make the "Diff Revision" label dynamically rendered in javascript.

Information

Reviewers