[viewvc-dev] Re: [viewvc-users] Don't display binary files

JJ <[email protected]> Wed, 12 Nov 2008 09:05:05 -0600
Newsgroups gmane.comp.version-control.cvs.viewcvs.devel
Message-ID <[email protected]>
Here's a smaller patch against HEAD that just adds the ignore_svn_mimetype
option.

I think the next step is adding the correct logic to markup_or_annotate that
detects if the file is viewable (meaning it is a viewable image or some
other text file).  Since the file is already being opened at that time,
perhaps using something like http://code.activestate.com/recipes/173220/ and
setting a is_viewable variable would be best.  Then the template can check
this value and display some appropriate message about the file not being
viewable, similar to
http://viewvc.tigris.org/source/browse/viewvc/trunk/viewvc.org/favicon.ico?rev=1946&view=markup
.

On Tue, Nov 11, 2008 at 11:21 AM, JJ <[email protected]> wrote:

>  On Tue, Nov 11, 2008 at 10:30 AM, C. Michael Pilato <[email protected]>wrote:
>
>> JJ wrote:
>> > 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.
>>
>> Well, there is *some* value, in that (as for text files) you can see the
>> most recent change metadata associated with the image, and you also have
>> handy links to other views of that thing (which can don't get with the
>> download view).
>>
>
>
> You could make the case that ANY file should be viewable then, since any
> file can have properties.
>
> In any event, the templates need some way of telling if the file is binary
> or text (i.e. something other than prefer_markup, since prefer_markup could
> then be set for text files or images) so they don't allow diffs or other
> things that don't make sense for binary 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).
>>
>> Hrm.  I wonder if the change to make ViewVC honor the svn:mime-type
>> property
>> is going to cause widespread annoyance because so many people haven't
>> taken
>> the time to set that property correctly?  Those folks will, in ViewVC 1.1,
>> lose the ability to see their files in the browser altogether.
>>
>> Do you think this behavior should be configurable?  ignore_svn_mimetype =
>> 0?
>>
>
>
> Yes, I personally would like this option.  I didn't realize this behaviour
> was new to 1.1.  If someone did enable it they should still be informed that
> the guessed mime type will not take the property into account.
>
>
>>
>> > 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.
>>
>> ViewVC can't afford to be any slower. :-)
>
>
> Agreed.
>
>
>>
>>
>> One comment about your patch:  I had mentioned that get_file_view_info()
>> would probably be handy as you built out the somewhat standard set of
>> links
>> for each file in view_revision(), but I see that you didn't take advantage
>> of that function.  Why?  Did it not meet your needs in some way I didn't
>> expect?
>>
> Probably because I don't know what I'm doing and wasn't listening very
> well.  :-(
>
> Another thought on detecting if a mime type is for a text file...
>
> This is ugly, but for experimentation sake I expanded is_text as follows.
>
> def is_text(mime_type):
>   rtn = not mime_type or mime_type[:5] == 'text/'
>   if not rtn:
>     from pygments.lexers import ClassNotFound, get_lexer_for_mimetype
>     try:
>       lexer = get_lexer_for_mimetype(mime_type)
>       rtn = True
>     except ClassNotFound:
>       pass
>   return rtn
> This works pretty well, though it is specific to the use of Pygments.
> Perhaps there is a more specific module in Python that can detect text files
> based on the mime type?  It looked like the mimetypes module can't, but
> perhaps I'm wrong.
>
> JJ
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
patch.txt (text/plain, 1.3 KB)
Index: viewvc.conf.dist
===================================================================
--- viewvc.conf.dist    (revision 2047)
+++ viewvc.conf.dist    (working copy)
@@ -269,6 +269,13 @@
 # Possible values: "tar", "annotate", "co", "markup", "roots"
 allowed_views = markup, annotate, roots

+# Don't use the svn:mime-type property to determine how to display a
+# file in the markup view.  This is helpful when, for example, an image
+# has svn:mime-type set to application/octet-stream.  If ViewVC tries
+# to display the image using the mime type application/octet-stream,
+# the image will look unrecognizeable.
+ignore_svn_mimetype = 0
+
 # authorizer: The name of the ViewVC authorizer plugin to use when
 # authorizing access to repository contents.  This value must be the
 # name of a Python module addressable as vcauth.MODULENAME (most
Index: lib/viewvc.py
===================================================================
--- lib/viewvc.py       (revision 2047)
+++ lib/viewvc.py       (working copy)
@@ -1465,7 +1465,8 @@
   mime_type = None
   if not path_parts:
     return None
-  if request.roottype == 'svn':
+  if request.roottype == 'svn' and \
+     not request.cfg.options.ignore_svn_mimetype:
     try:
       itemprops = request.repos.itemprops(path_parts, rev)
       mime_type = itemprops.get('svn:mime-type')