Hi!
> > Woah, so much new code... do we really need all of this? Have you evaluated > using pyxdg? Maybe it handles cross-platform? Thanks for pointing out pyxdg ... i was not aware of it. If the package for it is present in major distributions then we can use that code too. I doubt it is cross-platform though because Windows and OSX does not use XDG specification(s). I am trying to compile a list of these differences so that we can make mypaint as cross-platform as possible (I know these things do not cause problems with execution but at the end even these small things matter) > > And I'm a bit surprised that you had to modify SConstruct. Those changes are > not related to XDG support, mainly just for mypaint.c, right? Actually most of the changes were installation related. > >> + for path in self.datapaths: >> + themedir_src = join(path, 'desktop/icons') >> + theme.prepend_search_path(themedir_src) > > Doesn't that reverse the order? Is this intentional, or does it not matter > for some reason? You are right. I think I just copied some of the old code and adapted it. I can fix that. > >> - self.pixmaps = PixbufDirectory(join(self.datapath, 'pixmaps')) >> + self.pixmaps = PixbufDirectory(self.datapaths) > > Nice. > >> + def data_file(self, f): >> + for dp in self.datapaths: >> + if (os.path.exists(join(dp, f))): >> + return join(dp, f) >> + return None > > Seems a good idea to have such a method. Please don't put extra brackets > around the condition in python. Sorry. Force of habit (C and Java) > >> class PixbufDirectory: >> [...] >> def __getattr__(self, name): >> if name not in self.cache: >> - pixbuf = gdk.pixbuf_new_from_file(join(self.dirname, name + >> '.png')) >> - self.cache[name] = pixbuf >> + for tdir in self.dirnames: >> + tdir = join(tdir, 'pixmaps') >> + if os.path.exists(join(tdir, name + '.png')): > > Couldn't we call the data_file method instead? Perhaps. As far as I remember this part I just tried to adapt the old code in a meaningful way. > >> +++ b/gui/backgroundwindow.py >> [...] >> - open(os.path.join(self.user_brushpath, 'order_default.conf'), >> 'w').write(data) >> + open(os.path.join(self.confpath, 'order_default.conf'), >> 'w').write(data) > > Look like you moved order*.conf out of the brushes/ directory. Then you > should also rename it to brush_order*.conf. Personally I think I like to > have it in the brushes directory, so you can just copy this directory to get > the same brush collection, and erase the remaining GUI configuration > separately. But I guess it's fine either way. It is still in brushes subdirectory but in .config/mypaint. It was the easiest thing to do. I can do more specific manipulation if I would have a list of all possible config files at the moment. But I agree that in some cases it would be better to have config and brushes together. I was just trying to follow the specification the way I understood it. > >> --- /dev/null >> +++ b/mypaint.c >> [...] >> + char launchscript[1024], searchpath[256]; >> + token = strtok(lookup, ":"); >> + while (token != NULL) > > Python string handling is so nice, why bother with C here? If it is only for > a better process name in "top", then I think I'm against this. Actually most of this stuff can be moved to wrapped python code. I am willing to maintain the executable for as long as I will be involved with mypaint. But I have also found a python bug where it was mentioned that a cross-platform process name changing is being integrated into core library so the executable may not be needed anymore in a year or even sooner. > >> (tested on my Ubuntu 9.10 machine). >> Skeleton is ready to make it Windows compatible, but I would like to >> have some feedback on that from windows developers. > > Bakhtiar / zakkum / tumagonx <[email protected]> (all the same > person I think ;-) is the one to talk to here. He is sometimes on IRC. Great. I will write him an email. > > I think on Windows we basically know where things like GTK are installed and > we can expect the users not to bother messing around with custom > installation paths, so I wouldn't to put too much effort into doing > something different there. So far it seems to have worked fine by simply > doing the same thing as on Linux. And I would feel a bit bad about pushing > a git version that is known to break on Windows. I agree ... that is why I would like to make it windows compatible before this gets into master. > >> def get_paths_linux(): >> [... 73 lines of code ...] >> + >> +def get_paths_xp(): >> + #TODO: implement >> + raise Exception("Unsupported system") >> + >> +def get_paths_vista(): >> + #TODO: implement >> + raise Exception("Unsupported system") > > I don't think xp and vista (and 7) differ that much. And couldn't we maybe > make the same code work for all systems? I don't like maintaining much > special code for a platform that I can't test... It is not the code - it can be made very general ... basically just determining where to store config files. xp, vista and 7 have different directories for this (at least according to some google chrome page). Once this directory is in the code there is not much maintaining to do. The XDG specification is the most complicated in my opinion as there are different folders for different files. > >> I would appreciate if somebody would take a few minutes and test it out. > > Disclaimer: I have not tested it yet. Hehe ... no problem. > >> - # note: some distros use lib64 instead, they have to edit this... >> - lib_compiled='lib/mypaint/' >> [...] >> + for lpath in ['lib', 'lib32', 'lib64']: > > Also a looks like a hack, but looks better than the one before :-) Indeed. I have no idea how to make this more general. At the end there are always some conventions about where to find things that have to be hard-coded. > >> A few notes: >> http://wiki.mypaint.info/index.php?title=User:Lukacu/Storage > > Maybe I should add that order.conf and the brush files go together, so > order.conf might be a data file...? Suggestions about what goes where are welcome. I have deliberately simplified things so that somebody will say something ;) > >> * The old configuration directory is kept intact. mypaint now migrates >> the data from it ... if it exists (it should also copy files if they >> were modified in the old directory) If any files are copied to wrong >> destination that is probably because I have made some wrong >> assumptions. > > Hm... what about moving the files, but only if the new configuration > directory does not exist yet? What use cases did you consider? I expect > that a non-functional ~/.mypaint might also lead to confusion. If all > solutions seem equally bad, I suggest we do the one that requires the fewest > lines of code (eg. migrate nothing at all?) That was also one of the options. In the end I decided that the best thing to do at the moment is to have it copy data every time because it is not yet in master so people would just use this code to try things out ... that was my idea at least. But in the end the best idea would be to just delete the old dir if all files would be copied correctly (an extra step, but it has to be done just in case). > >> * I have found out, that scons does not install icons correctly (on my >> computer at least) and corrected this with some additional code. > > Strange, I thought we fixed that. Please review this to see if everything is > consistent with your changes: > http://gitorious.org/mypaint/mypaint/commit/cd4e89ff1e4a I can check it out yes. The problem is that the recursive copy did not work with Glob so I just wrote my own routine to do this. Of course using standard api would be the way to go (if it works). > >> (I have also noticed that this SConstruct is really Linux biased when it >> comes to installing files (but have no plan of correcting this on my own)) > > Yes, I think it is used only for building on Windows. > hm ... On Linux or on windows? I guess installation on windows is done using nsis. thanks for checking out the code :) -- Luka Čehovin http://luka.tnode.com _______________________________________________ Mypaint-discuss mailing list [email protected] https://mail.gna.org/listinfo/mypaint-discuss
