Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

Binary browse and image rev #534

Merged
merged 3 commits into from
May 10, 2016
Merged

Conversation

kleintom
Copy link
Contributor

@kleintom kleintom commented May 5, 2016

  • Allow browsing to binary files.
  • Display the nav pane for binary images and support viewing binary images on /rev/.
  • Support viewing of textual images on /rev/.

@kleintom kleintom force-pushed the binary_browse branch 2 times, most recently from 0723093 to ac13d8c Compare May 5, 2016 15:22
@kleintom
Copy link
Contributor Author

kleintom commented May 5, 2016

(Still needs a little more work :-)

* Copy 'icons/raw.png' to a new mimetype 'binary.png' and use it for binary
  files we don't otherwise know the mimetype of, mostly to make it easy to
  recognize binaries in /browse/ and search results now that they're not greyed
  out.

* Remove is_binary from JS results processing, and, since that's the only place
  it was used, remove it from query results as well.
@@ -249,6 +251,23 @@ def raw(tree, path):
return send_file(data_file, mimetype=guess_type(path)[0])


@dxr_blueprint.route('/<tree>/rawrev/<revision>/<path:path>')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not in love with the user-visible name "rawrev", since nowhere else do we mash two words up against each other without any divider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raw_rev?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far we've managed to keep underscores out of the URL, which is a nice thing, I think (esp. when we're already using dashes). I haven't had a chance to look through your PR in detail yet, but I vaguely and nebulously wonder if we really need this extra endpoint. I suppose it's to distinguish a binary's containing page from its raw data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit I was unaware of the dashes-vs-underscores debate, but even had I been, we do use dashes in filters as you point out, so dashes are more consistent - thanks!

Yes, the new endpoint is to serve up just the plain raw binary of a file at a particular revision rather than embedded in the dxr layout. Then the /rev/ page for a binary image can have a /raw-rev/ img href to the image at that rev (so the rev image gets displayed on its /rev/ page), and for svg /rev/ pages we can put a "view image" link in the nav pane to the svg served up as svg by a /raw-rev/.

@kleintom
Copy link
Contributor Author

kleintom commented May 5, 2016

@pelmers Do you have some time to look this one over?

…rev/.

* Add a '/<tree>/raw-rev/<revision>/<path:path>' route so that we can link
  to past revisions of binary image files in the output served by /rev/.

* Turn on omniglot links for binary files in general.
@kleintom
Copy link
Contributor Author

kleintom commented May 6, 2016

Updated to use 'raw-rev'.

@pelmers
Copy link
Contributor

pelmers commented May 9, 2016

Looks and works good to me, r+

Update: actually let me rescind that for a minute, I want to test something out real quick.

Update update: r+

@kleintom kleintom merged commit 7302c56 into mozilla:master May 10, 2016
@kleintom kleintom deleted the binary_browse branch May 10, 2016 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants