[compiz] Annotate, guiding line patches with questions

[David Reveman]

On Sat, 2007-01-06 at 01:47 +0100, lowfi wrote:
> Mike Dransfield schrieb:
> > I would like to have a small app written in python/gtk
> > which would be triggered with a keystroke.  The user
> > would hit the keystroke and the application would be
> > launched by the annotate plugin.
> > 
> > It could then provide a basic palette and text entering
> > functions (copy/paste, selecting font, style, color).  When
> > it is activated it can  change the action for  annotate so
> > the modifiers are removed, the user can then annotate
> > whatever they like, without worrying about modifiers
> > etc (an arrow tool will be provided which can be used
> > to move/select windows).
> 
> Great, this sounds exactly like what the new Vista Snipping Tool does.
> It has a lot of cool features and i already thought about how to
> implement some of them into compiz.
> 
> > When they are finished they can click a button which
> > would activate screenshot so that they can then take a
> > screenshot.
> > 
> > The screenshot plugin would then save the file and send
> > a signal through dbus to the helper app.  This can then
> > pick up the file and launch an email application or anything
> > else which can handle the image (instant messenger/web
> > forum etc etc).
> 
> Another approach could be to add an option to the screenshot plugin
> which launches an external app. This could launch your helper app or
> something else. I'm using this to upload screenshots to my flickr account.
> 
> Here is a patch for screenshot.c which adds a launch_app option.
> What do you think?

Seems like a simple and useful solution. Some comments to the patch:

allocating storage for the command line using:

+ comm = malloc (sizeof(app) + sizeof(dir) + sizeof(name) + 2);

is not going to work well as "app" and "dir" are just pointers and the
size of the memory that they are pointing to is not known at compile
time. You also need to make sure that you allocate enough memory to
store the terminating `\0' character.

I would change that line to:

+ comm = malloc (strlen (app) + strlen (dir) + strlen (name) + 3);

You should use:

runCommand (s, comm);

instead of calling execl directly. runCommand does things like exporting
the DISPLAY environment variable and setting the session id.

I don't like to have "eog" as the default. I'd rather like to see it be
set to nothing by default.


Please update your patch so I can include it.

Thanks,

-David