Re: [compiz] Flat file backend (ini)
On Thu, 2007-02-22 at 16:01 +, Mike Dransfield wrote: > I have written an initial flat file configuration > backend for Compiz. At the moment it is using > FAM for monitoring of files (it can be compiled > without monitoring support for now). Great! > > It is working fine but there are a couple of problems. > > 1. It looks like I should use addFileWatch rather than > directly accessing FAM, is this correct? I notice dbus > uses addFileWatch rather than (*d->addFileWatch) is > this intentional, and does it get wrapped in the same > way? Yep, you should use the file watch interface instead of FAM. You should call addFileWatch to add a watch for a specific file or directory. addFileWatch will then call display->fileWatchAdded which is a wrapped function that plugins that provide file watch functionality like the inotify function will hook into. If you have any problems with using the file watch interface please let me know. > > 2. I cannot see how to work out what the defaults are > so that people can revert to default values. As far as > I know Compiz does not store the original value so the > only way to revert back when someone deletes a config > file is to reload Compiz. I could keep the defaults in a > separate system wide file, but that would probably have > problems. I like having it only rely on user config files. > Would adding some default storage to the core make sense?, > we could then have a dbus method which would revert an > individual value easily. I'm not sure. I'd like to make it easy to adjust the defaults while packaging and do so differently for different configuration systems. It makes a lot of sense to have different default settings for e.g. gnome and kde. This is possible right now by patching the gconf schema file during packaging. Why do you think a system wide file would cause problems? It would allow the same kind of adjustments to the defaults during packaging as the schema file is currently doing, which is nice. > > > http://www.anykeysoftware.co.uk/compiz/plugins/ini.c > http://www.anykeysoftware.co.uk/compiz/plugins/Makefile.ini > > Any other comments or suggestions are welcome How about getting this into the git.compiz.org or the freedesktop.org repo? - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
David Reveman wrote: Yep, you should use the file watch interface instead of FAM. You should call addFileWatch to add a watch for a specific file or directory. addFileWatch will then call display->fileWatchAdded which is a wrapped function that plugins that provide file watch functionality like the inotify function will hook into. If you have any problems with using the file watch interface please let me know. I will write a fam plugin as well and test if that works correctly too. I think it will need extending a bit but I didn't look too hard. I'm not sure. I'd like to make it easy to adjust the defaults while packaging and do so differently for different configuration systems. It makes a lot of sense to have different default settings for e.g. gnome and kde. This is possible right now by patching the gconf schema file during packaging. Why do you think a system wide file would cause problems? It would allow the same kind of adjustments to the defaults during packaging as the schema file is currently doing, which is nice. My idea was that there would be a core function which could revert an individual option value to its default value, this could be copied only when it changes to save memory. This would mean that it would be easy to expose it via dbus for the different settings managers, plus it would mean that the gconf and ini (and any others in the future) would not have to know how to revert the defaults. Maybe we could add a compiz_defaults.h and a plugin_defaults.h file which would contain all the defaults in one easy place. Packagers would be able to patch this same file differently for each package. I know they can already do this, but moving everything to 1/2 files would encourage it. This raises another question I forgot about, how are multiple screens handled? ie. how does the schema file get modified depending on how many screens are being used? http://www.anykeysoftware.co.uk/compiz/plugins/ini.c http://www.anykeysoftware.co.uk/compiz/plugins/Makefile.ini Any other comments or suggestions are welcome How about getting this into the git.compiz.org or the freedesktop.org repo? I think it would be nice to include this one in the freedesktop.org repo because its more of a core plumbing plugin than an extra. I'd like to work out all the bugs first though, I am having a problem with edges and the wall plugin. It is working when I use gconf, so it must be my problem. The only difference is that I incrementally add to the action option whereas gconf does it in one hit. Do you think this would cause a problem? The edges work for other plugins (ie. scale) ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
It is working fine but there are a couple of problems.
1. It looks like I should use addFileWatch rather than
directly accessing FAM, is this correct? I notice dbus
uses addFileWatch rather than (*d->addFileWatch) is
this intentional, and does it get wrapped in the same
way?
Yep, you should use the file watch interface instead of FAM. You should
call addFileWatch to add a watch for a specific file or directory.
addFileWatch will then call display->fileWatchAdded which is a wrapped
function that plugins that provide file watch functionality like the
inotify function will hook into. If you have any problems with using the
file watch interface please let me know.
Hi,
I modified the inotify plugin to send an event structure instead of just
the file name in the callback. The event contains an event mask to allow to
switch between event types in the callback, and the file name.
Currently the name variable in the callback sends the event->name of the
inotify struct,
but that's not very useful. eg. if you watch two files directly, there is no
way to distinguish
between them in the callback, because event->name will be empty.
So the new CompNotifyEvent will send the watch path if a file is watched,
or the relative filename if a directory is watched.
I think this can be very useful for the ini plugin and for other plugins as
well.
Any thoughts on that?
diff --git a/plugins/inotify.c b/plugins/inotify.c
index 9782c61..e3d7a8b 100644
--- a/plugins/inotify.c
+++ b/plugins/inotify.c
@@ -53,6 +53,20 @@ typedef struct _InotifyDisplay {
#define INOTIFY_DISPLAY(d) \
InotifyDisplay *id = GET_INOTIFY_DISPLAY (d)
+static int
+inotifyNotifyEventMask (int inotifyMask)
+{
+int compMask = 0;
+
+if (inotifyMask & IN_CREATE)
+ compMask = NOTIFY_CREATE_MASK;
+else if (inotifyMask & IN_DELETE)
+ compMask = NOTIFY_DELETE_MASK;
+else if (inotifyMask & IN_MOVE)
+ compMask = NOTIFY_MOVE_MASK;
+
+return compMask;
+}
static Bool
inotifyProcessEvents (void *data)
@@ -90,7 +104,28 @@ inotifyProcessEvents (void *data)
break;
if (fw)
- (*fw->callBack) (event->name, fw->closure);
+{
+CompNotifyEvent *nev;
+
+nev = malloc (sizeof (CompNotifyEvent));
+if (nev)
+{
+if (event->len == 0)
+{
+nev->name = fw->path;
+}
+else
+{
+nev->name = event->name;
+}
+
+nev->mask = inotifyNotifyEventMask (event->mask);
+
+(*fw->callBack) (nev, fw->closure);
+
+free (nev);
+}
+}
}
i += sizeof (*event) + event->len;
diff --git a/include/compiz.h b/include/compiz.h
index 9107270..e67a1e3 100644
--- a/include/compiz.h
+++ b/include/compiz.h
@@ -621,8 +621,13 @@ typedef Bool (*ImageToFileProc) (CompDisplay *display,
#define NOTIFY_DELETE_MASK (1 << 1)
#define NOTIFY_MOVE_MASK (1 << 2)
-typedef void (*FileWatchCallBackProc) (const char *name,
- void *closure);
+typedef struct _CompNotifyEvent {
+const char *name;
+intmask;
+} CompNotifyEvent;
+
+typedef void (*FileWatchCallBackProc) (CompNotifyEvent *event,
+ void*closure);
typedef struct _CompFileWatch {
struct _CompFileWatch *next;
diff --git a/plugins/dbus.c b/plugins/dbus.c
index 782ad24..00e7d9b 100644
--- a/plugins/dbus.c
+++ b/plugins/dbus.c
@@ -1518,8 +1518,8 @@ dbusSetScreenOptionForPlugin (CompScreen *s,
}
static void
-dbusSendPluginsChangedSignal (const char *name,
- void *closure)
+dbusSendPluginsChangedSignal (CompNotifyEvent *event,
+ void*closure)
{
CompDisplay *d = (CompDisplay *) closure;
DBusMessage *signal;
___
compiz mailing list
compiz@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Tue, 2007-02-27 at 19:24 +, Mike Dransfield wrote: > David Reveman wrote: > > Yep, you should use the file watch interface instead of FAM. You should > > call addFileWatch to add a watch for a specific file or directory. > > addFileWatch will then call display->fileWatchAdded which is a wrapped > > function that plugins that provide file watch functionality like the > > inotify function will hook into. If you have any problems with using the > > file watch interface please let me know. > > > I will write a fam plugin as well and test if that works > correctly too. I think it will need extending a bit but I didn't > look too hard. Do you get any benefit out of fam except that it provides polling for when inotify isn't available? If not, I might make more sense to add a simple polling based file watch plugin instead of one that uses fam. > > > > > I'm not sure. I'd like to make it easy to adjust the defaults while > > packaging and do so differently for different configuration systems. It > > makes a lot of sense to have different default settings for e.g. gnome > > and kde. This is possible right now by patching the gconf schema file > > during packaging. > > > > Why do you think a system wide file would cause problems? It would allow > > the same kind of adjustments to the defaults during packaging as the > > schema file is currently doing, which is nice. > > > > My idea was that there would be a core function which could > revert an individual option value to its default value, this could > be copied only when it changes to save memory. > > This would mean that it would be easy to expose it via dbus for > the different settings managers, plus it would mean that the > gconf and ini (and any others in the future) would not have to > know how to revert the defaults. I don't know, maybe some default defaults could be good to have but I'm not convinced. If you have both a gnome config utility that uses gconf and some other kde utility that uses dbus running at the same time you likely want different defaults for each of these utilities and the default defaults wouldn't be useful to them. If you necessarily don't want to keep the defaults in a separate file, you could keep a copy of the initial value of any option you change in the ini plugin. We could provide some utility function that will make that easy. > > Maybe we could add a compiz_defaults.h and a plugin_defaults.h > file which would contain all the defaults in one easy place. Packagers > would be able to patch this same file differently for each package. > I know they can already do this, but moving everything to 1/2 files > would encourage it. I'm not sure I understand. Should these headers be included in each plugin that like to know about the defaults? Seems like worse then the schema file to me as now you have files that need to be patched differently depending on which plugin they are included in. > > This raises another question I forgot about, how are multiple screens > handled? ie. how does the schema file get modified depending on how > many screens are being used? That's broken. Only defaults for screen 0 is currently provided in the schema file. I don't have a good solution for this. > > >> http://www.anykeysoftware.co.uk/compiz/plugins/ini.c > >> http://www.anykeysoftware.co.uk/compiz/plugins/Makefile.ini > >> > >> Any other comments or suggestions are welcome > >> > > > > How about getting this into the git.compiz.org or the freedesktop.org > > repo? > > > > I think it would be nice to include this one in the freedesktop.org > repo because its more of a core plumbing plugin than an extra. > I'd like to work out all the bugs first though, I am having a problem > with edges and the wall plugin. It is working when I use gconf, so it > must be my problem. Sure, you can do that first if you want. I don't mind putting it in right now, though. > > The only difference is that I incrementally add to the action option > whereas gconf does it in one hit. Do you think this would cause a > problem? The edges work for other plugins (ie. scale) Depends on how reading and writing of options is implemented in your ini plugin. It's always preferred to do atomic changes to options so I would suggest that we fix this at some point. - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
David Reveman wrote: On Tue, 2007-02-27 at 19:24 +, Mike Dransfield wrote: I will write a fam plugin as well and test if that works correctly too. I think it will need extending a bit but I didn't look too hard. Do you get any benefit out of fam except that it provides polling for when inotify isn't available? If not, I might make more sense to add a simple polling based file watch plugin instead of one that uses fam. The only benefit was it was the easiest thing to install and use at the time ;) I think I will just install inotify and change ini to support that, then we can have a look at an inotify alternative. My idea was that there would be a core function which could revert an individual option value to its default value, this could be copied only when it changes to save memory. This would mean that it would be easy to expose it via dbus for the different settings managers, plus it would mean that the gconf and ini (and any others in the future) would not have to know how to revert the defaults. I don't know, maybe some default defaults could be good to have but I'm not convinced. If you have both a gnome config utility that uses gconf and some other kde utility that uses dbus running at the same time you likely want different defaults for each of these utilities and the default defaults wouldn't be useful to them. If you necessarily don't want to keep the defaults in a separate file, you could keep a copy of the initial value of any option you change in the ini plugin. We could provide some utility function that will make that easy. Yes, I hadn't thought of this before. I am not bothered either way. The ini plugin probably does not need to have a ini-dump plugin because it writes out the internal values if it does not find a user config file. Maybe we could add a compiz_defaults.h and a plugin_defaults.h file which would contain all the defaults in one easy place. Packagers would be able to patch this same file differently for each package. I know they can already do this, but moving everything to 1/2 files would encourage it. I'm not sure I understand. Should these headers be included in each plugin that like to know about the defaults? Seems like worse then the schema file to me as now you have files that need to be patched differently depending on which plugin they are included in. My initial idea was that the packagers could modify the individual c files. Most of the plugins have their defaults at the top in a nice easy to read format. I just expanded this idea to pull all these defines to one header file and then all the plugins can include the same file. This might have been a stupid idea though. This raises another question I forgot about, how are multiple screens handled? ie. how does the schema file get modified depending on how many screens are being used? That's broken. Only defaults for screen 0 is currently provided in the schema file. I don't have a good solution for this. Now might be a good time to solve this skeleton once and for all. Have you thought about making it a compile time option? The other alternative might be to pull the plugin loading functions into a separate library, then we could create a small utility program which could load a plugin and then ask it to write its schema to a global location. Users could run this utility when their xorg configuration app changes the number of screens. This could possibly be expanded to some sort of daemon which just listens for notifications from X about new screens, then adds schemas when needed. The only difference is that I incrementally add to the action option whereas gconf does it in one hit. Do you think this would cause a problem? The edges work for other plugins (ie. scale) Depends on how reading and writing of options is implemented in your ini plugin. It's always preferred to do atomic changes to options so I would suggest that we fix this at some point. Yes, I will fix this and tidy it up a bit. Hopefully that will fix the problem, otherwise Ill try to get to the bottom of the edge bug. ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Wed, 2007-02-28 at 15:28 +, Mike Dransfield wrote: > David Reveman wrote: > > On Tue, 2007-02-27 at 19:24 +, Mike Dransfield wrote: > > > >> I will write a fam plugin as well and test if that works > >> correctly too. I think it will need extending a bit but I didn't > >> look too hard. > >> > > > > Do you get any benefit out of fam except that it provides polling for > > when inotify isn't available? If not, I might make more sense to add a > > simple polling based file watch plugin instead of one that uses fam. > > > > > > The only benefit was it was the easiest thing to install and use at > the time ;) > > I think I will just install inotify and change ini to support that, then > we can have a look at an inotify alternative. > > >> My idea was that there would be a core function which could > >> revert an individual option value to its default value, this could > >> be copied only when it changes to save memory. > >> > >> This would mean that it would be easy to expose it via dbus for > >> the different settings managers, plus it would mean that the > >> gconf and ini (and any others in the future) would not have to > >> know how to revert the defaults. > >> > > > > I don't know, maybe some default defaults could be good to have but I'm > > not convinced. If you have both a gnome config utility that uses gconf > > and some other kde utility that uses dbus running at the same time you > > likely want different defaults for each of these utilities and the > > default defaults wouldn't be useful to them. > > > > If you necessarily don't want to keep the defaults in a separate file, > > you could keep a copy of the initial value of any option you change in > > the ini plugin. We could provide some utility function that will make > > that easy. > > > > Yes, I hadn't thought of this before. I am not bothered either way. > The ini plugin probably does not need to have a ini-dump plugin > because it writes out the internal values if it does not find a user > config file. Yes, that seems simple and good enough. > > >> Maybe we could add a compiz_defaults.h and a plugin_defaults.h > >> file which would contain all the defaults in one easy place. Packagers > >> would be able to patch this same file differently for each package. > >> I know they can already do this, but moving everything to 1/2 files > >> would encourage it. > >> > > > > I'm not sure I understand. Should these headers be included in each > > plugin that like to know about the defaults? Seems like worse then the > > schema file to me as now you have files that need to be patched > > differently depending on which plugin they are included in. > > > > My initial idea was that the packagers could modify the > individual c files. Most of the plugins have their defaults at > the top in a nice easy to read format. Problem is that you have one set of plugins and one core that is shared between all your desktop environments but you want different defaults depending on which desktop environment you're using. So patching the source code files doesn't work. > > I just expanded this idea to pull all these defines to one header > file and then all the plugins can include the same file. This might > have been a stupid idea though. > > >> This raises another question I forgot about, how are multiple screens > >> handled? ie. how does the schema file get modified depending on how > >> many screens are being used? > >> > > > > That's broken. Only defaults for screen 0 is currently provided in the > > schema file. I don't have a good solution for this. > > > > Now might be a good time to solve this skeleton once and > for all. Have you thought about making it a compile time option? > > The other alternative might be to pull the plugin loading functions > into a separate library, then we could create a small utility program > which could load a plugin and then ask it to write its schema to a > global location. Users could run this utility when their xorg > configuration > app changes the number of screens. > > This could possibly be expanded to some sort of daemon which just > listens for notifications from X about new screens, then adds schemas > when needed. Sounds complicated. I think a simple solution like having the schema install procedure be able to install the default values for any number of screens. The packager would select how many screens to install schemas for. One is probably fine in most cases and I'm pretty sure almost no one uses more than four screens so those who are concerned about people using more than one screen could use something like that. (this is only a problem when using multiple screens and not normal multi-head as you then just have one X screen) > > >> The only difference is that I incrementally add to the action option > >> whereas gconf does it in one hit. Do you think this would cause a > >> problem? The edges work for other plugins (ie. scale) > >>
Re: [compiz] Flat file backend (ini)
Gerd Kohlberger wrote: I modified the inotify plugin to send an event structure instead of just the file name in the callback. The event contains an event mask to allow to switch between event types in the callback, and the file name. Currently the name variable in the callback sends the event->name of the inotify struct, but that's not very useful. eg. if you watch two files directly, there is no way to distinguish between them in the callback, because event->name will be empty. So the new CompNotifyEvent will send the watch path if a file is watched, or the relative filename if a directory is watched. I think this can be very useful for the ini plugin and for other plugins as well. Any thoughts on that? This seems roughly what I would need. I applied your patches and added IN_MODIFY since I need that too. I have a new version which compiles with those patches and has the match type. It is not sending the notification correctly, I am not sure if it is my fault or not. Modify does not work at all, and delete just passed through a mask of 0. http://www.anykeysoftware.co.uk/compiz/plugins/ini.c Once these changes are in head, I will have more of an effort to fix them. All that needs to be done is make the action options atomic, which might be messy. I do not think that it is the cause of the problem with wall, but Ill have a go. Actions are working for everything except that one. ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Tue, 2007-02-27 at 20:58 +0100, Gerd Kohlberger wrote: > >> It is working fine but there are a couple of problems. > >> > >> 1. It looks like I should use addFileWatch rather than > >> directly accessing FAM, is this correct? I notice dbus > >> uses addFileWatch rather than (*d->addFileWatch) is > >> this intentional, and does it get wrapped in the same > >> way? > > > > Yep, you should use the file watch interface instead of FAM. You should > > call addFileWatch to add a watch for a specific file or directory. > > addFileWatch will then call display->fileWatchAdded which is a wrapped > > function that plugins that provide file watch functionality like the > > inotify function will hook into. If you have any problems with using the > > file watch interface please let me know. > > > > Hi, > > I modified the inotify plugin to send an event structure instead of just > the file name in the callback. The event contains an event mask to allow to > switch between event types in the callback, and the file name. hm, you're currently suppose to use different call-back functions for this. e.g. if you want to know the difference between move or delete do: addFileWatch (d, path, NOTIFY_MOVE_MASK, fileMovedCallBackFunc, (void *) data); addFileWatch (d, path, NOTIFY_DELETE_MASK, fileDeleteCallBackFunc, (void *) data); instead of: addFileWatch (d, path, NOTIFY_MOVE_MASK | NOTIFY_DELETE_MASK, fileRemovedCallBackFunc, (void *) data); > > Currently the name variable in the callback sends the event->name of the > inotify struct, > but that's not very useful. eg. if you watch two files directly, there is no > way to distinguish > between them in the callback, because event->name will be empty. if you want to watch two different files you should currently use two different call-back functions. The name variable is intended for when you're watching directories. > > So the new CompNotifyEvent will send the watch path if a file is watched, > or the relative filename if a directory is watched. > > I think this can be very useful for the ini plugin and for other plugins as > well. I'm not sure we want to do these changes as unless I'm misunderstanding something the current interface provides the same functionality. What are the advantages of changing this way? - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Fri, 2007-03-02 at 13:35 +, Mike Dransfield wrote: > Gerd Kohlberger wrote: > > > > I modified the inotify plugin to send an event structure instead of just > > the file name in the callback. The event contains an event mask to > > allow to > > switch between event types in the callback, and the file name. > > > > Currently the name variable in the callback sends the event->name of > > the inotify struct, > > but that's not very useful. eg. if you watch two files directly, there > > is no way to distinguish > > between them in the callback, because event->name will be empty. > > > > So the new CompNotifyEvent will send the watch path if a file is watched, > > or the relative filename if a directory is watched. > > > > I think this can be very useful for the ini plugin and for other > > plugins as well. > > > > Any thoughts on that? > > This seems roughly what I would need. I applied your patches and added > IN_MODIFY since I need that too. Yes, we need a MODIFY bit. > > I have a new version which compiles with those patches and has the match > type. It is not sending the notification correctly, I am not sure if it > is my > fault or not. Modify does not work at all, and delete just passed through > a mask of 0. > > http://www.anykeysoftware.co.uk/compiz/plugins/ini.c > > Once these changes are in head, I will have more of an effort to fix them. > > All that needs to be done is make the action options atomic, which might > be messy. I do not think that it is the cause of the problem with wall, but > Ill have a go. Actions are working for everything except that one. OK, lets put it in head now. - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
David Reveman wrote:
On Fri, 2007-03-02 at 13:35 +, Mike Dransfield wrote:
This seems roughly what I would need. I applied your patches and added
IN_MODIFY since I need that too.
Yes, we need a MODIFY bit.
I am attaching a patch for that
I have also changed the notification functions as you said.
I now create a custom structure to pass through which contains
all the information that I need.
There are 2 major problems though:
1. If I add a delete and a notify function then the closure gets
mixed up. Everything works if I just add 1 but more than 1 and
they seem to interact with each other. I am creating the watches
with different functions, I just pass the same closure (line 1031).
2. The modify notification works, but only the first time. With
the current version I can modify a file once and I get the notification
and the option reloads, but if I modify the file a second time then
I get no notification.
http://www.anykeysoftware.co.uk/compiz/plugins/ini.c
>From a6ef6656910318fe2b1bfa750ac6cb41bf5ed8ef Mon Sep 17 00:00:00 2001
From: [EMAIL PROTECTED] <[EMAIL PROTECTED](none)>
Date: Sat, 3 Mar 2007 20:07:57 +
Subject: [PATCH] Added IN_MODIFY bit
---
include/compiz.h |1 +
plugins/inotify.c |3 +++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/compiz.h b/include/compiz.h
index 18c51bc..6542368 100644
--- a/include/compiz.h
+++ b/include/compiz.h
@@ -621,6 +621,7 @@ typedef Bool (*ImageToFileProc) (CompDisplay *display,
#define NOTIFY_CREATE_MASK (1 << 0)
#define NOTIFY_DELETE_MASK (1 << 1)
#define NOTIFY_MOVE_MASK (1 << 2)
+#define NOTIFY_MODIFY_MASK (1 << 3)
typedef void (*FileWatchCallBackProc) (const char *name,
void *closure);
diff --git a/plugins/inotify.c b/plugins/inotify.c
index 9782c61..92a30d7 100644
--- a/plugins/inotify.c
+++ b/plugins/inotify.c
@@ -114,6 +114,9 @@ inotifyMask (CompFileWatch *fileWatch)
if (fileWatch->mask & NOTIFY_MOVE_MASK)
mask |= IN_MOVE;
+if (fileWatch->mask & NOTIFY_MODIFY_MASK)
+ mask |= IN_MODIFY;
+
return mask;
}
--
1.5.0.1
___
compiz mailing list
compiz@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
David Reveman schrieb: hm, you're currently suppose to use different call-back functions for this. e.g. if you want to know the difference between move or delete do: addFileWatch (d, path, NOTIFY_MOVE_MASK, fileMovedCallBackFunc, (void *) data); addFileWatch (d, path, NOTIFY_DELETE_MASK, fileDeleteCallBackFunc, (void *) data); instead of: addFileWatch (d, path, NOTIFY_MOVE_MASK | NOTIFY_DELETE_MASK, fileRemovedCallBackFunc, (void *) data); Currently the name variable in the callback sends the event->name of the inotify struct, but that's not very useful. eg. if you watch two files directly, there is no way to distinguish between them in the callback, because event->name will be empty. if you want to watch two different files you should currently use two different call-back functions. The name variable is intended for when you're watching directories. So the new CompNotifyEvent will send the watch path if a file is watched, or the relative filename if a directory is watched. I think this can be very useful for the ini plugin and for other plugins as well. I'm not sure we want to do these changes as unless I'm misunderstanding something the current interface provides the same functionality. What are the advantages of changing this way? Yes the functionality is already there, but with this patch it's just more convenient to monitor more than just 2 or 3 files. I modified inotify while playing around with the ini plugin and do doing it with the current interface will get you lots of callback functions which isn't really necessary. e.g if you listen for 3 different events in 5 files, you would need 15 callbacks in a plugin. You don't have to include the patch, i just thought it might be useful, but it seems Mike has already an ini version which works with the current interface. Gerd. ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
Mike Dransfield schrieb: David Reveman wrote: On Fri, 2007-03-02 at 13:35 +, Mike Dransfield wrote: This seems roughly what I would need. I applied your patches and added IN_MODIFY since I need that too. Yes, we need a MODIFY bit. I am attaching a patch for that I have also changed the notification functions as you said. I now create a custom structure to pass through which contains all the information that I need. There are 2 major problems though: 1. If I add a delete and a notify function then the closure gets mixed up. Everything works if I just add 1 but more than 1 and they seem to interact with each other. I am creating the watches with different functions, I just pass the same closure (line 1031). 2. The modify notification works, but only the first time. With the current version I can modify a file once and I get the notification and the option reloads, but if I modify the file a second time then I get no notification. The IN_MODIFY bit isn't really useful for text files. If you watch the event sequence which get emited by inotify you'll see something like: 1) MOVE compiz.conf -> compiz.conf~ 2) MOVE compiz.conf~ -> compiz.conf 3) DELETE compiz.conf~ So if you watch compiz.conf directly and you open it in a text editor it gets moved. After that inotify seems to loose track of that file and the kernel will send an IN_IGNORED event, which automatically removes the watch. This is why modify only works the first time. I think the best solution would be to watch only the ~/.compiz/options dir and put all files in this directory. You could changed the filenames to something like -.conf to represent the different screens. Also the inotify plugin should probably handle those kernel events (like IN_UNMOUNT), because if watches get automatically removed by the kernel, it will confuse compiz internal list of watches. But i haven't really tested this, so might not be necessary. Gerd. ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
Gerd Kohlberger wrote: The IN_MODIFY bit isn't really useful for text files. If you watch the event sequence which get emited by inotify you'll see something like: 1) MOVE compiz.conf -> compiz.conf~ 2) MOVE compiz.conf~ -> compiz.conf 3) DELETE compiz.conf~ So if you watch compiz.conf directly and you open it in a text editor it gets moved. After that inotify seems to loose track of that file and the kernel will send an IN_IGNORED event, which automatically removes the watch. This is why modify only works the first time. I think the best solution would be to watch only the ~/.compiz/options dir and put all files in this directory. You could changed the filenames to something like -.conf to represent the different screens. Thanks for the advice, I have changed it to do exactly that and it now seems to be working perfectly for all editors that I have. It seems that I still need IN_MODIFY because without it I was only receiving notifications on the temp file. I think all the major problems are solved now, its just a case of tidying up here and there. I will probably need to add proper file locking but it shouldn't cause too many problems. What happens when a file is deleted will still need to be resolved (at the moment it just writes it back) http://www.anykeysoftware.co.uk/compiz/plugins/ini.c I have ifdef'd out the MODIFY bit so it will still compile without the IN_MODIFY patch. Regards Mike ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Sun, 2007-03-04 at 22:47 +0100, Gerd Kohlberger wrote: > David Reveman schrieb: > > hm, you're currently suppose to use different call-back functions for > > this. > > > > e.g. if you want to know the difference between move or delete do: > > > > addFileWatch (d, path, NOTIFY_MOVE_MASK, fileMovedCallBackFunc, (void *) > > data); > > addFileWatch (d, path, NOTIFY_DELETE_MASK, fileDeleteCallBackFunc, (void *) > > data); > > > > instead of: > > > > addFileWatch (d, path, NOTIFY_MOVE_MASK | NOTIFY_DELETE_MASK, > > fileRemovedCallBackFunc, (void *) data); > > > >> Currently the name variable in the callback sends the event->name of the > >> inotify struct, > >> but that's not very useful. eg. if you watch two files directly, there is > >> no way to distinguish > >> between them in the callback, because event->name will be empty. > > > > if you want to watch two different files you should currently use two > > different call-back functions. The name variable is intended for when > > you're watching directories. > > > >> So the new CompNotifyEvent will send the watch path if a file is watched, > >> or the relative filename if a directory is watched. > >> > >> I think this can be very useful for the ini plugin and for other plugins > >> as well. > > > > I'm not sure we want to do these changes as unless I'm misunderstanding > > something the current interface provides the same functionality. What > > are the advantages of changing this way? > > Yes the functionality is already there, but with this patch it's just more > convenient > to monitor more than just 2 or 3 files. > I modified inotify while playing around with the ini plugin and do doing it > with the > current interface will get you lots of callback functions which isn't really > necessary. > > e.g if you listen for 3 different events in 5 files, you would need 15 > callbacks in a plugin. Well if you need to handle all those cases differently, you need to dispatch into 15 different segments of code and whether that is done in the file watch interface or in your plugin shouldn't matter much. One call back can be convenient when you're doing almost the same thing for all those 15 cases and it makes it easy to share code. You can share just as much code the other way too, you just have to use more functions. If you want to use only one call back function and do the dispatch in your plugin then that's easily done by providing dispatch details in the user-data parameter. I like the current interface because it's simple but still efficient. Reporting exactly what caused the file watch to trigger might be trivial in the inotify plugin but it might make another file watch handler unnecessarily complex. > > You don't have to include the patch, i just thought it might be useful, but > it seems Mike has > already an ini version which works with the current interface. OK, good. If I'm presented with an example where the current interface definitely cause more trouble than necessary, then I'm of course interested in changing the interface. - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Sun, 2007-03-04 at 23:06 +0100, Gerd Kohlberger wrote: > Mike Dransfield schrieb: > > David Reveman wrote: > >> On Fri, 2007-03-02 at 13:35 +, Mike Dransfield wrote: > >> > >>> This seems roughly what I would need. I applied your patches and added > >>> IN_MODIFY since I need that too. > >>> > >> > >> Yes, we need a MODIFY bit. > >> > > > > I am attaching a patch for that > > > > I have also changed the notification functions as you said. > > I now create a custom structure to pass through which contains > > all the information that I need. > > > > There are 2 major problems though: > > > > 1. If I add a delete and a notify function then the closure gets > > mixed up. Everything works if I just add 1 but more than 1 and > > they seem to interact with each other. I am creating the watches > > with different functions, I just pass the same closure (line 1031). > > > > 2. The modify notification works, but only the first time. With > > the current version I can modify a file once and I get the notification > > and the option reloads, but if I modify the file a second time then > > I get no notification. > > > > The IN_MODIFY bit isn't really useful for text files. > > If you watch the event sequence which get emited by inotify you'll see > something like: > > 1) MOVE compiz.conf -> compiz.conf~ > 2) MOVE compiz.conf~ -> compiz.conf > 3) DELETE compiz.conf~ > > So if you watch compiz.conf directly and you open it in a text editor it gets > moved. > After that inotify seems to loose track of that file and the kernel will send > an IN_IGNORED event, > which automatically removes the watch. This is why modify only works the > first time. > > I think the best solution would be to watch only the ~/.compiz/options dir > and put all files in > this directory. You could changed the filenames to something like > -.conf to represent > the different screens. > > Also the inotify plugin should probably handle those kernel events (like > IN_UNMOUNT), because if watches get automatically > removed by the kernel, it will confuse compiz internal list of watches. But i > haven't really tested this, so might not be necessary. Yes, inotify plugin should deal with that. I left that out for later and any patches that would improve the current code would be greatly appreciated. - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Mon, 2007-03-05 at 18:03 +, Mike Dransfield wrote: > Gerd Kohlberger wrote: > > > > The IN_MODIFY bit isn't really useful for text files. > > > > If you watch the event sequence which get emited by inotify you'll see > > something like: > > > > 1) MOVE compiz.conf -> compiz.conf~ > > 2) MOVE compiz.conf~ -> compiz.conf > > 3) DELETE compiz.conf~ > > > > So if you watch compiz.conf directly and you open it in a text editor > > it gets moved. > > After that inotify seems to loose track of that file and the kernel > > will send an IN_IGNORED event, > > which automatically removes the watch. This is why modify only works > > the first time. > > > > I think the best solution would be to watch only the ~/.compiz/options > > dir and put all files in > > this directory. You could changed the filenames to something like > > -.conf to represent > > the different screens. > > Thanks for the advice, I have changed it to do exactly that and it now > seems to be working perfectly for all editors that I have. > > It seems that I still need IN_MODIFY because without it I was only > receiving notifications on the temp file. > > I think all the major problems are solved now, its just a case of tidying > up here and there. I will probably need to add proper file locking but > it shouldn't cause too many problems. What happens when a file is > deleted will still need to be resolved (at the moment it just writes it > back) Great! > > http://www.anykeysoftware.co.uk/compiz/plugins/ini.c > > I have ifdef'd out the MODIFY bit so it will still compile without the > IN_MODIFY patch. IN_MODIFY is already in head so you don't need that anymore. It looks good. I'd like to get this into head today but the header needs to be fixed first. Novell, Inc. clearly don't hold the copyright to this file. Do "s/Novell, Inc./Mike Dransfield/" if nothing else. Thanks, - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
Diogo Ferreira wrote: The current INI plugin seems to fail loading when there isn't a ~/.compiz dir. Since most users won't have it I believe it makes sense to mkdir it before trying to mkdir the sub-options directory. I would have done it but since you use the get dir function a lot it would mean changing the structure a little and since it's your plugin I think you know best how to change it accordingly. Ive been meaning to get some clarification on whether or not I can rely on .compiz existing. When I first wrote the ini plugin, I though I could. Its more a global directory so I do not create it, but I can if its required. Should I be creating this directory? OR should it be the responsibility of core to make sure this exists (it is defined there) ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
The current INI plugin seems to fail loading when there isn't a ~/.compiz dir. Since most users won't have it I believe it makes sense to mkdir it before trying to mkdir the sub-options directory. I would have done it but since you use the get dir function a lot it would mean changing the structure a little and since it's your plugin I think you know best how to change it accordingly. Regards, Diogo Ferreira On 2/22/07, Mike Dransfield <[EMAIL PROTECTED]> wrote: I have written an initial flat file configuration backend for Compiz. At the moment it is using FAM for monitoring of files (it can be compiled without monitoring support for now). It is working fine but there are a couple of problems. 1. It looks like I should use addFileWatch rather than directly accessing FAM, is this correct? I notice dbus uses addFileWatch rather than (*d->addFileWatch) is this intentional, and does it get wrapped in the same way? 2. I cannot see how to work out what the defaults are so that people can revert to default values. As far as I know Compiz does not store the original value so the only way to revert back when someone deletes a config file is to reload Compiz. I could keep the defaults in a separate system wide file, but that would probably have problems. I like having it only rely on user config files. Would adding some default storage to the core make sense?, we could then have a dbus method which would revert an individual value easily. http://www.anykeysoftware.co.uk/compiz/plugins/ini.c http://www.anykeysoftware.co.uk/compiz/plugins/Makefile.ini Any other comments or suggestions are welcome ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On 4/5/07, Mike Dransfield <[EMAIL PROTECTED]> wrote: Diogo Ferreira wrote: > The current INI plugin seems to fail loading when there isn't a > ~/.compiz dir. Since most users won't have it I believe it makes sense > to mkdir it before trying to mkdir the sub-options directory. > I would have done it but since you use the get dir function a lot it > would mean changing the structure a little and since it's your plugin > I think you know best how to change it accordingly. > Ive been meaning to get some clarification on whether or not I can rely on .compiz existing. When I first wrote the ini plugin, I though I could. Its more a global directory so I do not create it, but I can if its required. Should I be creating this directory? OR should it be the responsibility of core to make sure this exists (it is defined there) You're right, it should be the core I guess, since it's an overall directory and if each plugin which wants to use it needs check for its existence it becomes a big amount of duplicated code. Maybe the core could create it on demand? Diogo ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Flat file backend (ini)
On Thu, 2007-04-05 at 19:50 +0100, Mike Dransfield wrote: > Diogo Ferreira wrote: > > The current INI plugin seems to fail loading when there isn't a > > ~/.compiz dir. Since most users won't have it I believe it makes sense > > to mkdir it before trying to mkdir the sub-options directory. > > I would have done it but since you use the get dir function a lot it > > would mean changing the structure a little and since it's your plugin > > I think you know best how to change it accordingly. > > > > Ive been meaning to get some clarification on whether > or not I can rely on .compiz existing. When I first wrote > the ini plugin, I though I could. > > Its more a global directory so I do not create it, but I can > if its required. > > Should I be creating this directory? OR should it be the > responsibility of core to make sure this exists (it is defined > there) There's no way we can ever guarantee that a ~/.compiz directory exists. ~/ could be read only or the disk could be full so make sure your code can deal with that properly. Whoever wants to write something to ~/.compiz should be responsible for trying to create the directory if it doesn't exist. - David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
