Re: Inital Deb Filter

[Joe Shaw <joeshaw novell com>]

Hi,

On Sun, 2006-04-16 at 01:13 -0400, Kevin Kubasik wrote:
> Ok, sorry It's been so long (boo Comcast) but here's the .deb filter
> with most of the changes we talked about. Some of the formatting stuff
> in HACKING wasn't super clear because of the odd syntax of this
> filter, if I missed anything else, please point it out, and I will be
> more than happy to fix it.

They're mostly whitespace related, I'll point them out as I go along.

> I also added a 'Package' tile for beagle-search. At the moment it
> doesn't display much extra, but it can easily be configured to show
> dependencies and the like. I was thinking about adding an 'Install'
> option in the context menu any thoughts on that?

A package tile really only makes sense if you can install them from the
UI.  This is one of those things that is so different from distro to
distro that I'm not sure we can effectively do this ourselves.  Plus, it
means additional space being used up in the main UI area.  I think just
leaving them in the file tile is probably the best course right now.

> This should be all you need to test it, I can't figure out how to get
> cvs to include new files in the diff, if anyone knows and wants to
> share, that would make this easier.

Usually you do "cvs add" on the file, and then "cvs diff" will show them
as new files.

Anyway, code comments here:

In your package filter, you have a bunch of FIXMEs about
internationalizing text.  In general, you can do that by importing the
Mono.Unix namespace, and using Catalog.GetString() on any strings that
should be marked for translation.  There is also GetPluralString() if
you need it.

All method calls and statements (like "if", "foreach", etc.) need to
have a space before the opening brace and spaces after commas.  There
are various things like: 

        AddProp("dc:title",tokens[1]);
        
They should be:

        AddProp ("dc:title", tokens [1]);
        
The same goes for assignments (a = b, not a=b) and comparisons (a == b,
not a==b).

> //
> // FilterDeb.cs: Extracts basic information from .deb archives
> //
> // Written By: Kevin Kubasik <kevin kubasik net>
> //

This will need a copyright and a license blob before checkin.

> namespace Beagle.Filters{

Please add a space before opening braces.

>         public FilterDeb (){
>                 AddSupportedFlavor (FilterFlavor.NewFromMimeType ("application/x-deb"));
>                 SnippetMode = true;
>         }

For methods, the opening brace goes on its own line.  That's the only
time when we have it on its own line.

> while ((str = pout.ReadLine ()) != null) {
>      strMetaTag = null;
>      tokens = str.Split (':');
>      if (tokens.Length <= 1) continue;

Please put the continue on its own line.

>      switch (tokens[0].Trim()) {
>      case "Package":
>              AddProp("dc:title",tokens[1]);
>              break;
>      case "Maintainer":
>              list = tokens[1].Split('<');
>              AddProp("dc:author",list[0]);
>              AddProp("fixme:author_email",list[1].Replace('>',' '));
>              break;

Can you add a blank line between the end of a case block and the start
of the next one?

Onto the package tile:

> using Beagle.Daemon;

You can't use the Beagle.Daemon namespace in beagle-search, because we
don't have a dependency of the client onto the daemon's assemblies (at
least, not right now).  The barrier is mostly artificial, which is why
the code works, but at some point it might become more hardened.  Plus,
you don't really need it for anything (except GetSnippet, which I
comment on below.)

> Description = StringFu.FileLengthToString( Convert.ToInt64 ( hit["fixme:size"]+"00"));

First, it looks like your search and replace to add spaces was
backwards. :)  But more importantly, what's with adding two zeros to the
end of a numeric string?  If the size is off by 100x, that's a bug, and
we shouldn't be working around it.

> if(my_hit.FileInfo.Extension == ".rpm" && my_hit["dc:description"] != null){
>        string[] temp= new string[my_query.Text.Count];
>        my_query.Text.CopyTo(temp,0);
>                                 
>        details.AddLabelPair("Description:",
>                             SnippetFu.GetSnippet(temp , new StringReader(my_hit["dc:description"])),3,1);
>        return details;
> }

I think what you're trying to do here is ellipsize the text of the
description, which might be quite long, correct?  If so, this is the
wrong way to do it, because it calls into the daemon's private
assemblies.  (A lot of stuff marked "public" should probably be marked
"internal" in there.)  What you want is to create a label and then call
beagle-search's WidgetFu.EllipsizeLabel().

Thanks,
Joe