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

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