GNOME Bugzilla – Bug 719722
Add a couple of CSS demos
Last modified: 2013-12-31 08:25:54 UTC
Created attachment 263339 [details] [review] patch adding CSS demo Add accordion and basic demo
Review of attachment 263339 [details] [review]: Nice demos, thanks. ::: demos/gtk-demo/demos/Css/css_accordion.py @@ +60,3 @@ + + provider = Gtk.CssProvider() + css_file = Gio.File.new_for_path("demos/data/css_accordion.css") It would be nice to compute an absolute path for this based on the py file location. I tend to run the demos from the base pygobject folder and would like to keep this flexibility as the rest of the demo app does it: https://git.gnome.org/browse/pygobject/tree/demos/gtk-demo/demos/pixbuf.py?id=3.11.2#n87 ::: demos/gtk-demo/demos/Css/css_basics.py @@ +56,3 @@ + + bytes = open("demos/data/css_basics.css", "rb").read() + buffer.set_text (bytes) I hit a snag with this in Python 3, perhaps bytes.decode('utf-8') ? @@ +77,3 @@ + section.get_end_position()) + + # FIXME: this should return a GLib.GError instead it returns Perhaps add a reference to https://bugzilla.gnome.org/show_bug.cgi?id=712519 ? So if we later search the code for that bug number once it's fixed, we can fix this up as well. @@ +93,3 @@ + text = buffer.get_text(start, end, False) + + # FIXME: we cannot set the GError to NULL so it will always what about wrapping it in a try/except?
Created attachment 263494 [details] [review] Revised patch adding part of the CSS demo thanks for the review Simon, enclosed an updated patch. Do you mind to commit for me if it's OK, I'm not doing much development for quite some time and I don't have my ssh key installed on this machine. Thank you
Simon what about the absolute path for CSS files, when a CSS file is imported from another one the path is relative from the python source and not from the CSS that imports the second one (if I'm right), what do you think, should I use a GResource and compile the CSS files and friends with glib-compile-resources? As a side note, is there a reason for not shipping the demo inside the released tarballs? Thanks
(In reply to comment #3) > Simon what about the absolute path for CSS files, when a CSS file is imported > from another one the path is relative from the python source and not from the > CSS that imports the second one (if I'm right), what do you think, should I use > a GResource and compile the CSS files and friends with glib-compile-resources? Is it relative from the Python file or the CWD? Since the error signal is broken it is a bit hard to know what is going on. If it is the CWD, a hack might be to change the cwd before and after loading the CSS? It would be nice if CssProvider supported some time of "site root" setting but I couldn't find anything like that. In any event, if a compiled resource can work, I think that would also make the demo interesting. > As a side note, is there a reason for not shipping the demo inside the released > tarballs? No reason I'm aware of, I think this is a good idea and I recall Martin mentioning it as well. There is a very minor technical problem that the demo sub-folders need to be renamed without using a space character, "Tree View" for example would need to be moved to "TreeView".
Created attachment 263625 [details] [review] Revised patch using Gio.Resource and add a new css demo I've added a new css example and used Gio.Resource to store the files.
Created attachment 263906 [details] [review] patch adding CSS demo An (hopefully) better version that uses the GResource for the whole CSS demo, can be extended for the whole demo later on by adding all the resources to the compiled file. OK to push?
(In reply to comment #6) > Created an attachment (id=263906) [details] [review] > patch adding CSS demo > > An (hopefully) better version that uses the GResource for the whole CSS demo, > can be extended for the whole demo later on by adding all the resources to the > compiled file. > > OK to push? While GResource is cool for compiled applications, I think it is a bit annoying to have to compile a python demo.... It feels so un-pythonic. What about a dirty try,catch,finally around the resource loading that calls out to glib-compile-resources in the failure case.
(In reply to comment #7) > While GResource is cool for compiled applications, I think it is a bit annoying > to have to compile a python demo.... It feels so un-pythonic. I could be missing something as I don't know much about GResource. But at least the Accordion demo works without anything being compiled. Does GResource work without stuff being compiled into a binary?
(In reply to comment #6) > OK to push? I just gave the demos another run and I'm having issues with both "CSS Basics" and "CSS Theming...". They don't seem to do anything except bring up an editing window which the style described doesn't apply (font is not monospace and selection color is not darkGreen). Unfortunately I can't see errors due to the PyGObject GError bugs. I assume both of those styles are supposed to be applied to the editing window when it first comes up?
(In reply to comment #8) > (In reply to comment #7) > > While GResource is cool for compiled applications, I think it is a bit annoying > > to have to compile a python demo.... It feels so un-pythonic. > > I could be missing something as I don't know much about GResource. But at least > the Accordion demo works without anything being compiled. Does GResource work > without stuff being compiled into a binary? Ah, never mind. It looks like it is pre-compiled and added to the patch as demo.gresource
(In reply to comment #7) Hi John, > While GResource is cool for compiled applications, I think it is a bit annoying > to have to compile a python demo.... It feels so un-pythonic. the primary purpose of the demo is to show how to do things, and I think showing hot to use a GResource to ship compiled CSS (and maybe other stuff) it's important. > What about a dirty try,catch,finally around the resource loading that calls out > to glib-compile-resources in the failure case. all the files are compiled already in the gresource.demo file which should be shipped with the demo itself (it's a tiny file).
Created attachment 265030 [details] [review] Revised patch using Gio.Resource and add a new css demo Fixed text encoding to work with Python3
Created attachment 265040 [details] [review] Revised patch using Gio.Resource and add a new css demo As discussed with Simon on IRC, I've added a comment on how the GResource works, next step would be add a Makefile that compiles the resource (and that adds the demo to the releases).
Thanks for the updates! Pushed with a few PEP8 and PyFlakes fixes. Also updated blanket try/except silencing with a specific one for GError in the css-provider domain.