[viewvc-dev] Re: [viewvc-users] Don't display binary files
JJ <[email protected]> Mon, 10 Nov 2008 15:42:46 -0600
| Newsgroups | gmane.comp.version-control.cvs.viewcvs.devel |
|---|---|
| Message-ID | <eed30ed90811101342y3fc1dd5fte5bad1801a3e37e3__28507.7853048735$1226353469$gmane$org@mail.gmail.com> |
My patch is attached to this email. I made the following decisions, which you can feel free to disagree with. 1) I do not set prefer_markup to true for any images. There isn't much value in showing the image in the markup view as opposed to the download view. In addition it causes problems because things such as diff, annotate and download as text were enabled for image files. The other problem is that an image file may have prefer_markup set to true, but then when displaying it in the markup view it looks like garbled text if the svn:mime-type is set to something like application/octet-stream (something which is quite common). 2) I changed the default template to never link to the markup view if prefer_markup is false. 3) I changed the default template to never link to things that apply only to text files (diff, download as text, view) if prefer_markup is false. It seems like the check for whether it is a text file needs to be smarter now, to include more than just files with text/* mime types in this category. Opening the files would be a performance hit of course. Any thoughts? JJ On Thu, Nov 6, 2008 at 11:59 AM, JJ <[email protected]> wrote: > Per the thread below, I have some local changes working that add the > following variables to revision.ezt. > > changes.view_href > changes.log_href > changes.download_href > changes.download_text_href > changes.prefer_markup > changes.annotate_href > As discussed below, I did this because I need to check if a particular > change prefers to be marked up to determine if they should get the markup > view or the checkout view. This is to avoid binary files being displayed > with garbled characters in the markup view. > > prefer_markup is set (in my code and in the existing code) as follows. > > prefer_markup = default_view(mime_type, request.cfg) == view_markup > mime_type is set as follows. > > mime_type = mime_type or guess_mime(where) > This is fine, but this guessed mime type is later redefined based off the > value of svn:mime-type for the file. So prefer_markup may be set to true > based off the guess (e.g. for a .jpg file) but then if that file has a mime > type such as application/octet-stream, the image is once again displayed as > garbled characters. > > It seems like we either need to stop guessing the mime type and always > check the svn:mime-type property, or always guess and ignore the > svn:mime-type property when displaying the file in the markup view. > > What do you think? Any other ideas for how to solve this? > > Thanks, > JJ > On Tue, Nov 4, 2008 at 9:35 AM, C. Michael Pilato <[email protected]>wrote: > >> Well-written patches are always welcome. I suspect that the patch needed >> here is likely to make use of get_file_view_info(), if that gives you a >> leg >> up on it. Sadly, the revision view was added (by me, I admit) without a >> great deal of fore-thought. Hopefully that fact doesn't work too hard >> against the development of a solution here. >> >> (As an aside, we should probably move this conversation to dev@ if we >> continue along this line of discussion.) >> >> JJ wrote: >> > Let me know if there is anything I can do to assist. I'm not terribly >> > familiar with the code, but I know Python and Subversion and am quite >> > interested in getting these variables added and having an official >> > release before I move ViewVC into production where I work. >> > >> > If you want me to make the variables below (and any others you can think >> > of) available in every template and you point me in the general >> > direction in the code, I can try to produce a patch. >> > >> > JJ >> > >> > On Tue, Nov 4, 2008 at 9:21 AM, C. Michael Pilato <[email protected] >> > <mailto:[email protected]>> wrote: >> > >> > I'm basically finished with the stuff I'd planned to accomplish for >> > 1.1. I >> > branched the source tree for 1.1 stabilization last week (I think it >> > was). >> > I'd like to do some sanity testing and release a beta tarball this >> > week. My >> > ultimate goal is to have 1.1 released by the end of this calendar >> > year, but >> > time is flying so we'll see how that goes. >> > >> > JJ wrote: >> > > Great! Sorry to bother you again, but would mind answering my >> > question >> > > about release timing, if you have any idea when it will be? :-) >> > > >> > >> Also, do you have any idea when the 1.1 release will be? Once I >> get >> > >> these changes in place I'm going to do some performance testing >> on >> > >> ViewVC and then move this to production. >> > > >> > > On Tue, Nov 4, 2008 at 7:22 AM, C. Michael Pilato >> > <[email protected] <mailto:[email protected]> >> > > <mailto:[email protected] <mailto:[email protected]>>> >> wrote: >> > > >> > > Yes. There's a handful of data that I'd intended to be >> available >> > > everywhere: >> > > >> > > view_href >> > > log_href >> > > download_href >> > > download_text_href >> > > prefer_markup >> > > annotate_href >> > > >> > > and maybe some others. This is the set I'll be trying to >> > introduce >> > > into the >> > > revision view. >> > > >> > > JJ wrote: >> > > > Another thing that would be great is if the download_href >> > could be >> > > > accessible from revisions.ezt. My hope is to have the links >> to >> > > > individual files take me to file.ezt if prefer_markup is >> > true, and if >> > > > not link to the download_url. >> > > > >> > > > Would that be possible? >> > > > >> > > > Also, do you have any idea when the 1.1 release will be? >> > Once I get >> > > > these changes in place I'm going to do some performance >> > testing on >> > > > ViewVC and then move this to production. >> > > > >> > > > Thanks again. >> > > > JJ >> > > > >> > > > On Mon, Nov 3, 2008 at 6:59 PM, C. Michael Pilato >> > > <[email protected] <mailto:[email protected]> >> > <mailto:[email protected] <mailto:[email protected]>> >> > > > <mailto:[email protected] <mailto:[email protected]> >> > <mailto:[email protected] <mailto:[email protected]>>>> wrote: >> > > > >> > > > Hrm. I had intended for the log, directory, markup, >> > annotate, and >> > > > revision >> > > > views to all carry this type of information uniformly. >> If I >> > > missed the >> > > > revision view on this, that's unintended. I'll try to >> > fix this >> > > > before 1.1 >> > > > is released. >> > > > >> > > > >> > > > JJ wrote: >> > > > > I mean "I also need to check prefer markup", not >> "change >> > > > prefer_markup". >> > > > > >> > > > > On Mon, Nov 3, 2008 at 3:40 PM, JJ >> > > <[email protected] >> > <mailto:[email protected]> >> > <mailto:[email protected] <mailto: >> [email protected]>> >> > > > <mailto:[email protected] >> > <mailto:[email protected]> >> > > <mailto:[email protected] >> > <mailto:[email protected]>>> >> > > > > <mailto:[email protected] >> > <mailto:[email protected]> >> > > <mailto:[email protected] >> > <mailto:[email protected]>> >> > > > <mailto:[email protected] >> > <mailto:[email protected]> >> > > <mailto:[email protected] >> > <mailto:[email protected]>>>>> wrote: >> > > > > >> > > > > Sorry for the delay in replying. >> > > > > >> > > > > It looks like that approach will work, except that >> I >> > > also need to >> > > > > change prefer_markup from revision.ezt, but that >> > is not an >> > > > available >> > > > > variable there. Do you know of another option, or >> > will this >> > > > require >> > > > > a code change to ViewVC? >> > > > > >> > > > > Thanks, >> > > > > JJ >> > > > > >> > > > > >> > > > > On Mon, Oct 13, 2008 at 9:48 AM, C. Michael Pilato >> > > > > <[email protected] <mailto:[email protected]> >> > <mailto:[email protected] <mailto:[email protected]>> >> > > <mailto:[email protected] <mailto:[email protected]> >> > <mailto:[email protected] <mailto:[email protected]>>> >> > > > <mailto:[email protected] <mailto:[email protected] >> > >> > <mailto:[email protected] <mailto:[email protected]>> >> > > <mailto:[email protected] <mailto:[email protected]> >> > <mailto:[email protected] <mailto:[email protected]>>>>> wrote: >> > > > > >> > > > > JJ wrote: >> > > > > >> > > > > Thanks for the reply. >> > > > > >> > > > > I have a couple questions. >> > > > > >> > > > > 1) I'm experimenting with file.ezt, >> > having it check >> > > > > mime_type before deciding if the file >> > should be >> > > displayed. >> > > > > Is there a pattern match option in ezt? >> > I can do >> > > > something >> > > > > like... >> > > > > >> > > > > >> > > > > Why are you playing with file.ezt at all? If >> > a user >> > > requests >> > > > > the markup or annotate view of a file, ViewVC >> > should >> > > try to >> > > > > display such. I think what you want instead >> > to do is to >> > > > change >> > > > > things like directory.ezt and log.ezt to not >> > link to the >> > > > > markup/annotate view for files for which >> > > prefer_markup is "0". >> > > > > >> > > > > -- >> > > > > C. Michael Pilato <[email protected] >> > <mailto:[email protected]> >> > > <mailto:[email protected] <mailto:[email protected]>> >> > > > <mailto:[email protected] <mailto:[email protected] >> > >> > <mailto:[email protected] <mailto:[email protected]>>> >> > > <mailto:[email protected] <mailto:[email protected]> >> > <mailto:[email protected] <mailto:[email protected]>> >> > > > <mailto:[email protected] <mailto:[email protected] >> > >> > <mailto:[email protected] <mailto:[email protected]>>>>> >> > > > > CollabNet <> www.collab.net >> > <http://www.collab.net> >> > > <http://www.collab.net> <http://www.collab.net> >> > > > <http://www.collab.net> <> >> > > > > Distributed Development On Demand >> > > > > >> > > > > >> > > > > >> > > > >> > > > >> > > > -- >> > > > C. Michael Pilato <[email protected] >> > <mailto:[email protected]> >> > > <mailto:[email protected] <mailto:[email protected]>> >> > <mailto:[email protected] <mailto:[email protected]> >> > > <mailto:[email protected] <mailto:[email protected]>>>> >> > > > CollabNet <> www.collab.net <http://www.collab.net> >> > <http://www.collab.net> >> > > <http://www.collab.net> <> >> > > > Distributed Development On Demand >> > > > >> > > > >> > > >> > > >> > > -- >> > > C. Michael Pilato <[email protected] >> > <mailto:[email protected]> <mailto:[email protected] >> > <mailto:[email protected]>>> >> > > CollabNet <> www.collab.net <http://www.collab.net> >> > <http://www.collab.net> <> >> > > Distributed Development On Demand >> > > >> > > >> > >> > >> > -- >> > C. Michael Pilato <[email protected] <mailto:[email protected] >> >> >> > CollabNet <> www.collab.net <http://www.collab.net> <> >> > Distributed Development On Demand >> > >> > >> >> >> -- >> C. Michael Pilato <[email protected]> >> CollabNet <> www.collab.net <> Distributed Development On Demand >> >> > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
patch.txt
(text/plain, 12.4 KB)
Index: templates/log.ezt
===================================================================
--- templates/log.ezt (revision 2045)
+++ templates/log.ezt (working copy)
@@ -22,21 +22,27 @@
Revision [is roottype "svn"]<a href="[entries.revision_href]"><strong>[entries.rev]</strong></a>[else]<strong>[entries.rev]</strong>[end] -
[if-any entries.view_href]
[is pathtype "file"]
- (<a href="[entries.view_href]">view</a>)
+ [if-any entries.prefer_markup]
+ (<a href="[entries.view_href]">view</a>)
+ [end]
[else]
<a href="[entries.view_href]">Directory Listing</a>
[end]
+ [is pathtype "file"]
+ [if-any entries.download_href](<a href="[entries.download_href]">download</a>)[end]
+ [if-any entries.download_text_href][if-any entries.prefer_markup](<a href="[entries.download_text_href]">as text</a>)[end][end]
+ [if-any entries.annotate_href][if-any entries.prefer_markup](<a href="[entries.annotate_href]">annotate</a>)[end][end]
+ [end]
[end]
- [if-any entries.download_href](<a href="[entries.download_href]">download</a>)[end]
- [if-any entries.download_text_href](<a href="[entries.download_text_href]">as text</a>)[end]
- [if-any entries.annotate_href](<a href="[entries.annotate_href]">annotate</a>)[end]
[is pathtype "file"]
- [# if you don't want to allow select for diffs then remove this section]
- [is entries.rev rev_selected]
- - <strong>[[]selected]</strong>
- [else]
- - <a href="[entries.sel_for_diff_href]">[[]select for diffs]</a>
+ [if-any entries.prefer_markup]
+ [# if you don't want to allow select for diffs then remove this section]
+ [is entries.rev rev_selected]
+ - <strong>[[]selected]</strong>
+ [else]
+ - <a href="[entries.sel_for_diff_href]">[[]select for diffs]</a>
+ [end]
[end]
[end]
[end]
@@ -105,42 +111,44 @@
[is entries.state "dead"]
<br /><strong><em>FILE REMOVED</em></strong>
[else]
- [is pathtype "file"]
- [if-any entries.prev]
- <br />Diff to <a href="[entries.diff_to_prev_href]">previous [entries.prev]</a>
- [if-any human_readable]
- [else]
- (<a href="[entries.diff_to_prev_href]&diff_format=h">colored</a>)
- [end]
- [end]
+ [is pathtype "file"]
+ [if-any entries.prefer_markup]
+ [if-any entries.prev]
+ <br />Diff to <a href="[entries.diff_to_prev_href]">previous [entries.prev]</a>
+ [if-any human_readable]
+ [else]
+ (<a href="[entries.diff_to_prev_href]&diff_format=h">colored</a>)
+ [end]
+ [end]
- [is roottype "cvs"]
- [if-any entries.branch_point]
- , to <a href="[entries.diff_to_branch_href]">branch point [entries.branch_point]</a>
- [if-any human_readable]
- [else]
- (<a href="[entries.diff_to_branch_href]&diff_format=h">colored</a>)
+ [is roottype "cvs"]
+ [if-any entries.branch_point]
+ , to <a href="[entries.diff_to_branch_href]">branch point [entries.branch_point]</a>
+ [if-any human_readable]
+ [else]
+ (<a href="[entries.diff_to_branch_href]&diff_format=h">colored</a>)
+ [end]
+ [end]
[end]
- [end]
- [if-any entries.next_main]
- , to <a href="[entries.diff_to_main_href]">next main [entries.next_main]</a>
- [if-any human_readable]
- [else]
- (<a href="[entries.diff_to_main_href]&diff_format=h">colored</a>)
+ [if-any entries.next_main]
+ , to <a href="[entries.diff_to_main_href]">next main [entries.next_main]</a>
+ [if-any human_readable]
+ [else]
+ (<a href="[entries.diff_to_main_href]&diff_format=h">colored</a>)
+ [end]
[end]
- [end]
- [end]
- [if-any entries.diff_to_sel_href]
- [if-any entries.prev], [else]<br />Diff[end]
- to <a href="[entries.diff_to_sel_href]">selected [rev_selected]</a>
- [if-any human_readable]
- [else]
- (<a href="[entries.diff_to_sel_href]&diff_format=h">colored</a>)
+ [if-any entries.diff_to_sel_href]
+ [if-any entries.prev], [else]<br />Diff[end]
+ to <a href="[entries.diff_to_sel_href]">selected [rev_selected]</a>
+ [if-any human_readable]
+ [else]
+ (<a href="[entries.diff_to_sel_href]&diff_format=h">colored</a>)
+ [end]
+ [end]
[end]
[end]
- [end]
[end]
<pre class="vc_log">[entries.log]</pre>
Index: templates/include/log_footer.ezt
===================================================================
--- templates/include/log_footer.ezt (revision 2045)
+++ templates/include/log_footer.ezt (working copy)
@@ -1,7 +1,17 @@
[include "paging.ezt"]
+[define prefer_markup][end]
+
[is pathtype "file"]
- [include "diff_form.ezt"]
+ [for entries]
+ [if-any entries.prefer_markup]
+ [define prefer_markup]1[end]
+ [end]
+ [end]
+
+ [if-any prefer_markup]
+ [include "diff_form.ezt"]
+ [end]
[end]
[include "sort.ezt"]
Index: templates/include/log_header.ezt
===================================================================
--- templates/include/log_header.ezt (revision 2045)
+++ templates/include/log_header.ezt (working copy)
@@ -23,10 +23,10 @@
<tr>
<td>Links to HEAD:</td>
<td>
- (<a href="[head_view_href]">view</a>)
+ [if-any head_prefer_markup](<a href="[head_view_href]">view</a>)[end]
[if-any head_download_href](<a href="[head_download_href]">download</a>)[end]
- [if-any head_download_text_href](<a href="[head_download_text_href]">as text</a>)[end]
- [if-any head_annotate_href](<a href="[head_annotate_href]">annotate</a>)[end]
+ [if-any head_prefer_markup][if-any head_download_text_href](<a href="[head_download_text_href]">as text</a>)[end][end]
+ [if-any head_prefer_markup][if-any head_annotate_href](<a href="[head_annotate_href]">annotate</a>)[end][end]
</td>
</tr>
[end]
Index: templates/revision.ezt
===================================================================
--- templates/revision.ezt (revision 2045)
+++ templates/revision.ezt (working copy)
@@ -61,11 +61,11 @@
[if-any changes]
[for changes]
<tr class="vc_row_[if-index changes even]even[else]odd[end]">
- <td>[if-any changes.view_href]<a href="[changes.view_href]" title="View [is changes.pathtype "dir"]Directory[else]File[end] Contents">[end]<img src="[docroot]/images/[is changes.pathtype "dir"]dir[else]text[end].png" class="vc_icon" alt="Directory" />[changes.path][is changes.pathtype "dir"]/[end][if-any changes.view_href]</a>[end]
+ <td>[if-any changes.prefer_markup][if-any changes.view_href]<a href="[changes.view_href]" title="View [is changes.pathtype "dir"]Directory[else]File[end] Contents">[end][end]<img src="[docroot]/images/[is changes.pathtype "dir"]dir[else]text[end].png" class="vc_icon" alt="Directory" />[changes.path][is changes.pathtype "dir"]/[end][if-any changes.prefer_markup][if-any changes.view_href]</a>[end][end]
[if-any changes.is_copy]<br /><em>(Copied from [changes.copy_path], r[changes.copy_rev])</em>[end]
</td>
<td>[if-any changes.log_href]<a href="[changes.log_href]" title="View Log">[end][changes.action][if-any changes.log_href]</a>[end]
- [if-any changes.text_mods], [if-any changes.diff_href]<a href="[changes.diff_href]" title="View Diff">[end]text changed[if-any changes.diff_href]</a>[end][end]
+ [if-any changes.text_mods], [if-any changes.prefer_markup][if-any changes.diff_href]<a href="[changes.diff_href]" title="View Diff">[end][end]text changed[if-any changes.prefer_markup][if-any changes.diff_href]</a>[end][end][end]
[if-any changes.prop_mods], props changed[end]
</td>
</tr>
Index: lib/viewvc.py
===================================================================
--- lib/viewvc.py (revision 2045)
+++ lib/viewvc.py (working copy)
@@ -14,7 +14,7 @@
#
# -----------------------------------------------------------------------
-__version__ = '1.2-dev'
+__version__ = '1.1.0-beta1'
# this comes from our library; measure the startup time
import debug
@@ -987,8 +987,8 @@
# very useful marked up. If the mime type is totally unknown (happens when
# we encounter an unrecognized file extension) we also view it through
# the markup page since that's better than sending it text/plain.
- if ('markup' in cfg.options.allowed_views and
- (is_viewable_image(mime_type) or is_text(mime_type))):
+ if ('markup' in cfg.options.allowed_views and is_text(mime_type)):
+ #(is_viewable_image(mime_type) or is_text(mime_type))):
return view_markup
return view_checkout
@@ -3183,7 +3183,8 @@
# Add the hrefs, types, and prev info
for change in changes:
- change.view_href = change.diff_href = change.type = change.log_href = None
+ change.view_href = change.diff_href = change.type = change.mime_type = change.log_href = \
+ change.download_href = change.download_text_href = change.annotate_href = change.prefer_markup = None
# If the path is newly added, don't claim text or property
# modifications.
@@ -3210,6 +3211,8 @@
link_rev = str(rev)
link_where = path
+ change.mime_type = guess_mime(link_where)
+
change.view_href = request.get_url(view_func=view_func,
where=link_where,
pathtype=change.pathtype,
@@ -3221,15 +3224,43 @@
params={'pathrev' : link_rev},
escape=1)
- if change.pathtype is vclib.FILE and change.text_changed:
- change.diff_href = request.get_url(view_func=view_diff,
- where=path,
- pathtype=change.pathtype,
- params={'pathrev' : str(rev),
- 'r1' : str(rev),
- 'r2' : str(change.base_rev),
- },
- escape=1)
+ if change.pathtype is vclib.FILE:
+ if 'co' in request.cfg.options.allowed_views:
+ change.download_href = request.get_url(view_func=view_checkout,
+ where=link_where,
+ pathtype=change.pathtype,
+ params={'pathrev' : link_rev},
+ escape=1)
+ if not is_plain_text(change.mime_type):
+ change.download_text_href = request.get_url(view_func=view_checkout,
+ where=link_where,
+ pathtype=change.pathtype,
+ params={'content-type': 'text/plain',
+ 'pathrev' : link_rev},
+ escape=1)
+
+ if 'annotate' in request.cfg.options.allowed_views:
+ change.annotate_href = request.get_url(view_func=view_annotate,
+ where=link_where,
+ pathtype=change.pathtype,
+ params={'annotate': link_rev,
+ 'pathrev': link_rev},
+ escape=1)
+
+ change.prefer_markup = ezt.boolean(default_view(change.mime_type, request.cfg) == view_markup)
+
+ if change.text_changed:
+ change.diff_href = request.get_url(view_func=view_diff,
+ where=path,
+ pathtype=change.pathtype,
+ params={'pathrev' : str(rev),
+ 'r1' : str(rev),
+ 'r2' : str(change.base_rev),
+ },
+ escape=1)
+ else:
+ # Directories never prefer markup
+ change.prefer_markup = ezt.boolean(0)
# use same variable names as the log template