Re: [Tracker] XMP/CC in Tracker, first patch

[jamie <jamiemcc blueyonder co uk>]

On Thu, 2007-06-07 at 20:26 -0700, Jason Kivlighn wrote:
> Hey Jamie,
>
> I'm done with school now and have lots of time to devote to this.
>
> For quick questions, is IRC a good place to ask?  Is there a particular
> time that you're more likely to be on?
>
> Attached is work I've done so far, diff'ed against trunk.  It looks for
> .filename.xmp in the current directory, and if it finds it runs the
> extractor on it.  At the moment, only the cc:license field is recognized.
>
> It also updates the pdf extractor to read xmp from the pdf's metadata
> field.  The current Poppler glib bindings don't allow reading the
> metadata field, so that had to be patched to make it work.  I've emailed
> the Poppler mailing list about this.
>
> Configure.ac is updated to check for the exempi library
> (http://www.figuiere.net/hub/blog/?Exempi) and conditionally compile in
> support for xmp.
>
> Overall, I've spend the last few weeks poking around in the code and
> trying to get familiar with how Tracker works.  I imagine I'll focus
> down on one thing at a time as I get comfortable with the overall picture.
>
> How does the patch look?  Style?  Approach? etc...  Also, how do I go
> about getting commit access?

A few issues with it

1) coding style is almost correct. Need to make suyre theres always a
space between function name and opening bracket. And no spaces within
the bracket

 gchar *sidecar_uri =
g_build_filename(sidecar_dir,g_strconcat(".",sidecar_basename,".xmp",NULL),NULL);

the above is wrong style - should be space between g_build_filename and
opening brackett


2) the above also includes memory leaks

g_strconcat returns an allocated string that was not freed (you should
never embed calls like g_strconcat within another function as you cant
free the memory)

3) Are we going to be using "." as the filename prefix? I thought we
should do as Hub suggested ?


Other than that its fine

Are you happy to fix the above?


jamie