hi Till On Fri, Jun 11, 2010 at 05:56:32PM +0200, Till Hartmann wrote: > attached are two patches, [...]
Please resend the (updated) patches in the form of git commits. The command "git format-patch origin/master" will produce the correct files. > the first of them being the merge-layer-button.patch which was a > "suggested task" I found here > http://wiki.mypaint.info/Development/JuniorJobs . Code looking good. Haven't tried yet but it looks like it will work :-) > I don't know if there's an easy pixbuf method which will only scale down, > but not up. There is helpers.pixbuf_thumbnail() in MyPaint which you could use, and there is similar but not identical code when saving the ORA thumbnail (probably for no good reason). > The file-preview.patch also contains little modifications to > the mimetype set to be used by the "recent_manager" (hence "and a half" > [is there an easy way to split patches?]). Probably many (just google it), but I always struggle a bit when I have to do this so I can't reccommend you one. If you keep it in a single patch, just mention it in the commit message. > + if extension == ".ora": > + mimetype = 'image/openraster' > + else: > + try: > + mimetype = mimetypes.types_map[extension] > + except: > + mimetype = 'application/octet-stream' > gtk.recent_manager_get_default().add_full("file://" + > self.filename, Better Python style would be to do this without exception handlig, like this: mimetype = mimetypes.types_map.get(extension, 'application/octet-stream') But... we are saving our own file here. We should really /know/ its mime type. Maybe just use a hardcoded dictionary mapping our well-known extensions to their mime type (or can they change depending on the system?). But more importantly we do want to know when something goes wrong here. We want to see an exception and not silently use 'application/octet-stream' the next time somebody adds a new format. > + def update_preview_cb(self, file_chooser, preview): > + filename = file_chooser.get_preview_filename() > + pixbuf = self.get_preview_image(filename) > + preview.set_from_pixbuf(pixbuf) > + file_chooser.set_preview_widget_active(pixbuf != None) > + return The return statement is redundant and should be removed. set_preview_widget_active(pixbuf) might work (using the truth value of pixbuf) but I haven't tried. Anyway, you might want to look up the difference between (pixbuf != None) and (pixbuf is not None). It doesn't really matter here, just something you should understand while you're learning more Python. > + def get_preview_image(self, filename): > + if filename: > + if os.path.splitext(filename)[1] == ".ora": > + #TODO check for upper-/lowercase variants You can use: os.path.splitext(filename)[1].lower() == ".ora" (Not sure if other code in mypaint also accepts uppercase, but it can't hurt.) > + return pixbuf > + else: > + try: > + pixbuf = gtk.gdk.pixbuf_new_from_file_at_size(filename, > 256, 256) > + return pixbuf > + except: > + return None > + return Both "return" and "return None" do the same thing, and neither of them is neccessary because it's the default thing Python does at the end of a function. (Use "pass" for doing nothing.) > diff --git a/gui/layerswindow.py b/gui/layerswindow.py > index 73db4b9..a080bf3 100644 > --- a/gui/layerswindow.py > +++ b/gui/layerswindow.py > @@ -361,17 +361,20 @@ class Window(windowing.SubWindow): > add_button = stock_button(gtk.STOCK_ADD) > move_up_button = stock_button(gtk.STOCK_GO_UP) > move_down_button = stock_button(gtk.STOCK_GO_DOWN) > + merge_down_button = stock_button(gtk.STOCK_DND_MULTIPLE) Hm, that stock icon may change appearance to something else. But never mind, should be good enough for now and can be fixed when it screws up. Rest of your patches look fine to me. -- Martin Renold _______________________________________________ Mypaint-discuss mailing list [email protected] https://mail.gna.org/listinfo/mypaint-discuss
