|
|||||||||||
|
Re: [long] An attempt to improve the code of the GTK+ frontend
From: Attilio Fiandrotti <attilio.fiandrotti(at)gmail.com>
Date: Tue Jul 24 2007 - 09:13:08 EDT
>> I never knew that it would take me so much time, but I now consider >> that the work I had started on improving the code of the GTK+ frontend >> of cdebconf is ready for a broader review. > > Thanks for all the work Jérémy! > > I'll not comment on the really technical side of the changes, mostly > because my grasp of C is not sufficient to have an opinion on them. > However, in general the refactoring seems like a good idea to me. Indeed, the code is technically well written / formatted >> --- 8< --- >> >> We could use the following process to integrate it: >> * discuss, comment and test; > > Hmm. If the changes are so structural, I'm not sure that the effort of > creating patches to have a gradual changeover is worth it. I'd suggest > just getting the code in your branch into the state we want it and then > just merging it into trunk in one commit. Probably, because of the huge number of changes, a single commit is a more viable option
>> I am open to normalize the code before integrating it. But I would
>> be inclined to get a broader opinion about at least:
>>
>> } else {
> [...] >> I find the former much more readable! > > I agree that this would be an improvement. Colin? I agree: code in GTK frontend is not very clean and needed some reworking >> 2.3 Code has been split in different files > >> The longest file have a total of 638 lines (including documentation >> and header) vs. 1790 lines for the current code. > > The stats at the bottom show an increase in lines of code of 1000 lines, > or 70%. That seems excessive, but I suspect the true increase is more > subtle. > > Could you provide sloccounts: > - excluding comments > - excluding completely new functionality (such as the entropy plugin > > That would give a more realistic basis for comparison. > >> 2.5 Separation of code specific to the debian-installer > >> The screenshot module is compiled conditionally as I see no reason >> to enable this feature on standard desktops. > > Agreed.
I see that in current jeremy's proposal the screenshot button is
disabaled by default: i think we must somehow allow the user to take
screenshots using the mouse only.
>> 3.5 fe_gtk_set_title() reactivated >> >> The implementation of the set_title method of cdebconf was changed >> to the default one in r43372 to avoid spurious title changes during >> pkgsel. > > Note that the TITLE command is deprecated in debconf anyway, although > there is at least one location in D-I (lowmem) where it is currently used > and, AFAICT, needed. Because this looks like it's more related to the generic debconf architecture, i cannot properly speak about it. >> 4.1 Banner adapts to various screen size >> >> The banner now adapts to any screen width. The logo is aligned on >> the left of the screen, and its right side is filled with the color >> defined in LOGO_BACKGROUND_COLOR. > > I'm not sure that this is a good solution. With the current bannersetting > a fixed background color works, but this would not be true for more > complex banners as proposed in #414785 or [1], or with banners for > derived distributions using a completely different banner (such a Dzongha > Linux [2]). > > Basically, hardcoding a color seems very wrong to me. fjp's observation are correct, probably a better solution has to be seeked. >> 4.2 Main window adapts to various screen size >> >> WINDOW_HEIGHT and WINDOW_WIDTH constants were removed, as the screen >> height and width are are now dynamically retrieved through >> gdk_screen_get_height() and gdk_screen_get_width(). The frontend now >> works a bit better on higher-resolution [11]. > > IMO supporting higher resolutions will require a redesign of the > interface, for the following reasons: > - the text area becomes too wide, which means that lines of text become > too long which reduces readability > - the buttons get very isolated at the extreme bottom of the screen > > In other words: yes, supporting higher resolutions is a great goal, but > not without making sure that the total presentation is still useable. IMO > we'd need to limit the effective usable area of the screen, for example > by defining a rectangular container within which all other elements are > positioned. > We'd probably also need a somewhat more interesting background outside > that container than our current grey. Probably a good idea would be taking advantage of hi-resolution framebuffers (like PPC's) by using bigger fonts, so that the screen is not left empty because of small size fonts. Of course this would require some experimenting and usability testing. > [Next to sections switched] >> 4.4 Go Back as secondary button >> >> The Go Back button was set secondary. In GTK+ terminology, this >> means that it will be displayed on the opposite side of other buttons >> added to the action box. >> >> This make the frontend looks a bit more like the newt frontend. I am >> not sure if it's better on a usability standpoint, though. > > I don't think it is, mainly because the buttons are now much too far > apart. I agree: the back and continue buttons should be close one to each other >> 4.3 Screenshot button has been removed >> >> To be able to make more experiments with the interface, the >> screenshot button has been removed. F10 can be pressed to optain the >> same effect. The code to create a button is still in screenshot.c >> though, but commented out. > > I don't think F10 is a good button to use for that as it is completely > non-intuitive. It would be nice to have the PrtScreen button as an > alternative way to activate a screenshot. Currently PrtSc does take a screenshot in ppm format, but the problem in taking screenshots using a key is that not all architectures have same keys (e.g.: on PPCs there is no PrtSc key) and, some embedded deviced may simply have no keyboard at all, but just a pointing device. >> Frans suggested on IRC that it could perfectly be an option in a >> menu. I would find it great, except that I don't really have an idea on >> where we could put it. :) > > Yes, I do think that having a menu in D-I would be a perfect option. > It could contain not only the screenshot button, but for example also > the "extra" menu items (CD integrity check, Save debug logs, Change > debconf priority, etc.). > > I am against removing the screenshot button while we do not have an > alternative method that is *visible* to the user. > However, making the screenshot button "secondary" and moving it to the > other side of the screen does seem like a good option to me. I agree on both points ith fjp (menu and screenshot button) >> 4.5 Yes/No buttons for boolean questions >> >> When a boolean question is the only question in a GO, Yes and No >> buttons are used instead of radio buttons to get the answer. >> >> A screenshot is available [12] if you want to take a look on the >> result. > > Looking at the screenshots, I don't think this is an improvement over > using the radio buttons. Again, the main reason is distance; in this case > distance from the question. > In the newt frontend, the buttons are immediately below the question. > Here, you have to make a visual journey of more that 20 cm (on my laptop) > to link the question with the buttons... > > Again, not a bad option, but would require a significant redesign of how > dialogs are displayed. IIRC, the radio option was preferred over both pulldown menu and buttons . More important, in current design GtkButtons are specifically reserved for back/continue/screenshot butotns, so i suggest to never use GtkButtons somewhere else. >> 4.6 glib logs to syslog >> >> Instead of being lost, logs written by GTK+ are now going to the >> global syslog. I hope this will allow us to track problems more >> easily. > > Seems OK to me, especially since only real "errors" get redirected there, > so it does not completely clutter the syslog. ok i agree > Would it be possible to redirect the other messages on VT1 to a file too, >> 5.1 An entropy plugin for the GTK+ frontend >> >> In order to test that the public API for the plugin was sound, I >> implemented a GTK+ version of the entropy plugin [13] found in >> cdebconf-entropy. > > That's really excellent! Yes, that's something the GTK frontend lacks in order to be idempotent with newt, very good! <snip/> On the reworking of SELECT handlers, i would temporarily move into a separate file special handlers for countrychooser and partman, and latermove that part of code into an appropriate plugin. As i said before privately, i agree with re-designing the GTK frontend code the way you described, provided you can support the new codebase during Lenny release cycle. regards Attilio -- To UNSUBSCRIBE, email to debian-boot-REQUEST@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.orgReceived on Tue Jul 24 09:09:20 2007 This archive was generated by hypermail 2.1.8 : Thu Aug 09 2007 - 18:03:07 EDT |
||||||||||
|
|||||||||||