[RFC] GTK+ 3 Migration - Alpha-grade Patchset

[John Ralls]

> On Feb 22, 2016, at 9:56 AM, Tobias Markus <tobias at miglix.eu> wrote:
> 
> On So, 2016-02-21 at 13:22 -0800, John Ralls wrote:
>>> 
>>> On Feb 21, 2016, at 11:12 AM, Tobias Markus <tobias at miglix.eu>
>>> wrote:
>>> 
>>> Hi GnuCash developers,
>>> 
>>> over the last few feeks, I have developed a fundamental patchset
>>> that
>>> should get GnuCash up and running in GTK+ 3.
>>> 
>>> The source code is available at GitHub:
>>> https://github.com/hesiod/gnucash/tree/gtk3
>>> 
>>> Some independent commits are on the next branch. All GTK+ 3-related
>>> stuff is on the gtk3 branch.
>>> 
>>> I'm sorry for the sometimes messy commits, but the number of
>>> commits is
>>> quite massive already and I really didn't have the time splitting
>>> them
>>> up. If you have any questions regarding a particular commit, feel
>>> free
>>> to ask me.
>>> 
>>> DISCLAIMER: This is alpha-quality software and not at all ready for
>>> users! Please do not run GnuCash built from my development tree
>>> unless
>>> you want to contribute to the development process. This development
>>> version may delete all your data and eat your CPU!
>>> 
>>> While I got GnuCash pretty much working under GTK+ 3, a lot of
>>> further
>>> (fundamental and architectural) changes are required before even
>>> thinking about a release. Simply spoken, we now have GnuCash
>>> working in
>>> GTK+ 3, but using GTK+/Gnome 2 patterns. While it is possible to go
>>> down this path, the user experience will be substandard, especially
>>> compared to the current releases.
>>> 
>>> Because GnuCash 2.0 was (afaik) coupled to the GTK+ 2 migration, I
>>> suggest that the GTK+ 3 version should be GnuCash 3.0.
>>> 
>>> A quick word on dependencies: I set the minimum GTK+ version to be
>>> 3.18
>>> because I didn't try to compile it with any earlier versions, but
>>> backporting probably isn't too hard.
>>> 
>>> Major changes already in my tree:
>>> * All "rich" GtkActions/GtkRadioActions/GtkToggleActions were
>>> replaced
>>>   by abstract GActions/GMenus. This doesn't sound like a lot, but
>>> is a
>>>   major change. Please read the relevant API documentation and do
>>> not
>>>   hestitate to ask me if you have any questions about the new
>>> GAction-
>>>   based infrastructure. Sorry for not going into more detail,
>>>   but describing the technical peculiarities here would take
>>>   considerably too long.
>>> * I removed the splash screen and replaced it by GApplication's
>>> busy
>>>   notification in some places.
>>> * I updated the HTML code to use the Webkit2 API, but it needs some
>>>   more love.
>>> * Using GtkApplication renders everything related to
>>>   gtk-osx-integration unnecessary.
>>> * This is unrelated to the GTK+ 3 migration, but I added a Markdown
>>>   version of the README and added a DOAP project description.
>>> * I changed the action naming pattern from CamelCaseAction to
>>>   dot.separated.action-name, e.g. file.close or account.d
>>> 
>>> Major stuff not yet migrated:
>>> * In order to migrate the small business module, its entry ledger
>>> has  
>>>   to be rewritten, just like the "normal" register.
>>> * AqBanking uses the gwenhywfar UI library, which in turn uses GTK+
>>> 2.
>>>   I have not yet looked into this.
>>> * I removed the old file history/recent files code. There are
>>> better
>>>   ways to do this in GTK+ 3.
>>> * I haven't converted every UI definition file to the new format
>>> for
>>>   now.
>>> 
>>> Things that I recommend be dropped/removed for GnuCash 3.0:
>>> * Maintaining both the CMake and autotools-based build system does
>>> not
>>>   seem like a good idea, so drop autotools.
>>> * Rewriting the old register is not really viable or useful,
>>>   so drop it. Currently, either the old or the new register is
>>> compiled
>>>   to ease transition. (Right now, the new register can be enabled
>>> in
>>>   addition to the old register.)
>>> * The dynamic module system (based on GModule) is a nice idea, but
>>> the
>>>   
>>>   benefits are dubious at the moment. While the dynamic loading of
>>> all
>>>  
>>>   the standard modules incurs a heavy startup delay, there is no
>>>   reason why a user would not want to load all the built-in
>>> modules.
>>>   Therefore, all GnuCash modules should be converted into
>>> statically
>>>   linked objects. (It is still possible to compile shared libraries
>>>   for Guile at the same time, CMake should have no problem doing
>>>   that.)
>>> 
>>> Major Changes that I highly recommend for a GnuCash 3.0 release:
>>> * Writing GObject-based code (i.e. GTK+ code) in C is a very
>>> cumbersome
>>>   and boilerplate-ish process. Vala makes it possible to write
>>> GObject
>>>  
>>>   code in a very concise way, all while compiling directly to C
>>> source
>>>  
>>>   (and C headers), making it possible to seamlessy integrate Vala
>>> code
>>>   into C code. Other C-to-Vala ports have seen significant
>>> decreases in
>>>   
>>>   code size. Porting the UI code to Vala should definitiely ease
>>>   maintainability, correctness and readability without sacrificing
>>>   performance.
>>> * I ported the core application code to use GtkApplication. It
>>> would
>>> be 
>>>   nice to change the main window - opened file relationship from
>>> the
>>>   current N:1 to a 1:1, i.e. each main window is separate and has
>>>   exactly one opened file. This would simplify the main window
>>> code.
>>>   It also seems more intuitive than the current behaviour, since
>>> the
>>>   user does not expect that all windows change the opened file if
>>> he
>>>   opens a file in one window.
>>> * UI design: see below
>>> * I have not yet looked at translations and accelerators.
>>> * The new register requires some more love.
>>> 
>>> Nice-to-haves regarding GTK:
>>> * Some GSimpleActions would be better off as GPropertyActions.
>>> * Validate all GtkBuilder files using gtk-builder-tool.
>>> * File Chooser Dialog: Implement proper file format filter.
>>> * xdg-app support
>>> * jqplot as a Git submodule
>>> * Have hints for GtkEntries
>>> 
>>> Nice-to-haves, other that the UI:
>>> * Replace the GncInt128 with Boost.Multiprecision
>>> * I added initial configuration files for clang-format and clang-
>>> tidy.
>>>   The latter could be integrated as a Git commit hook.
>>> 
>>> Bugfixing paths:
>>> * I have added basic support for compiling GnuCash with the address
>>>   sanitizer (ASan) and the undefined behaviour sanitizer (UBSan),
>>>   although in practice, the former only works using LD_PRELOAD at
>>>   runtime (which is discouraged) due to a Guile error.
>>> * Many of the warnings you see compiling my tree really are valid
>>>   errors and should be fixed.
>>> 
>>> UI Design Notes:
>>> 
>>> The current UI has the menubar as the central element. Depending
>>> on    
>>> which "plugin page" you're currently looking at, certain menus are
>>> shown or hidden, e.g. the "Transaction" menu is only visible when
>>> viewing a register page. In theory, this behaviour is also
>>> available in
>>> my development tree thanks to the EggMenuManager (taken from Gnome
>>> Builder), but I removed the relevant calls for the moment. Please
>>> read
>>> on for the reasons. (Search for calls to egg_menu_manager_remove)
>>> 
>>> Both the heavy usage of menus and the context-sensitive visibility
>>> of
>>> menus are discouraged by the Gnome 3 HIG. A more modern UI design
>>> would
>>> greatly benefit the GnuCash user experience. (Before continuing,
>>> please
>>> have a quick look at the Gnome 3 HIG right now if you haven't yet.)
>>> 
>>> I haven't spent a great deal of time thinking about a potential new
>>> UI
>>> design yet, but in general, it boils down to having the page-
>>> specific
>>> actions as buttons inside a toolbar, and general actions in a
>>> header
>>> bar (GtkHeaderBar). As an example, let's take the register:
>>>  * New Account: A plus button on the toolbar
>>>  * Register Style: Clicking on "sandwich"-like button opens a
>>> popover
>>>    offering the different styles as a slider (as in Nautilus: the
>>>    button left of the sandwich button offers the different Nautilus
>>>    style options)
>>>  * View - Search by: As "search" button in the header bar
>>> 
>>> In general, you can get a lot of design inspiration by looking at
>>> Gnome
>>> Apps, in particular some of the recently refreshed ones, e.g.
>>> Files/Nautilus, Videos/Totem, Maps, Documents or Document
>>> Viewer/evince.
>>> 
>>> Asking the Gnome Design Team for suggestions is also a possible
>>> avenue.
>>> 
>>> Thanks for your time and attention - let's make GnuCash an even
>>> better
>>> free software finance manager!
>>> 
>> Dear Tobias,
>> 
>> That's a huge amount of work, but it's a bit too far-reaching to be
>> considered a patch set and it doesn't really look like something we
>> can merge.
> 
> Greetings John,
> 
> first of all, thanks for your comments. No Sweat - I'm not asking for a
> merge right now, this is really just a fundamental set of patches to
> get GnuCash running under GTK+ 3 and obviously needs more work.
> 
>> 
>> 
>> It's also quite possible that we won't ever move GnuCash to Gtk+-3.
>> We haven't decided on a future UI toolkit and if you'd looked at
>> http://wiki.gnucash.org/wiki/Roadmap you'd realize that we have other
>> priorities. Unfortunately it doesn't look like many of your changes
>> advance those priorities, and the way you've done your changes would
>> make it very difficult indeed to cherry-pick out the parts that might
>> be useful. 
> 
> Please have a look at the roadmap again :D There are only two
> paragraphs regarding the UI in the roadmap, and they are "Register
> rewrite" - my work is basically building on top of the register rewrite
> - and "Eliminate deprecated widgets and libraries" - that's what my
> patches are all about - going from GTK+ 2 to GTK+ 3, from webkitgtk to
> webkit2gtk, and replacing the deprecated Gnome Canvas based code! :)
> 
> Regarding the toolkits, I will respond in detail at the end of this
> reply.
> 
> About the non-GTK+3-ish changes: I've tried to keep some of the more
> general improvements on the next branch, but unfortunately, most of the
> changes are complex enough that cherry-picking them from the gtk3
> branch is possible, but somewhat complicated. Since the actual coding
> work was large enough, I concentrated on getting the initial migration
> ready.
> 
>> 
>> Had you come here before starting work we could have guided you
>> towards work that could be merged and coached you about making commit
>> messages, style, and the like. Perhaps you'd consider starting over?
> 
> If by starting over you mean doing some more rebasing to improve the
> commit messages, sure! But as I said, I would rather get my gtk3 branch
> feature-ready, i.e. implement the major points I mentioned above.
> Please let me know which changes were especially unclear or confusing,
> and I will happily improve their commit messages.
> 
> About the commit messages: I can do better, I promise :D
> As mentioned above, I focused on getting the branch ready.
> Most of the "Gnome WIP" commits are pretty similar actually, they all
> consist of the core GTK+ 3 migration changes, i.e. replacing deprecated
> features by their recommended replacements.
> 
> What exactly do you mean with style? I tried to have most of my
> additions follow the general style of the relevant file.
> 
>> 
>> 
>> About your recommendations:
>> * We probably will drop autotools in favor of cmake for the next
>> release.
>> 
>> * Vala is not useful to GnuCash. It is usable only with Gtk+ and we
>> need to be able to write GUIs for other platforms. We decided two
>> years ago to redo the guts in C++11 and are working on that.
> 
> John, I didn't mean to say that the entire GnuCash codebase should be
> written in Vala, that certainly wouldn't be the right way. My idea is
> to rewrite just the UI (i.e. GTK) code in Vala.
> 
> Don't get me wrong, I love C++ and C++11/14 most of all! I just thought
> that if the UI code is rewritten, and I think we both agree that the
> current UI code consists of a lot of boilerplate code, it should be
> rewritten in Vala. (But honestly, that's not a very strong opinion of
> mine, as I said, I really like C++ too.)
> 
> The reason why I suggested Vala instead of C++/gtkmm is that Vala is a
> 1:1 match to the GObject system, and while gtkmm code is certainly
> easier to write that pure GTK+/C code, they aren't really a perfect
> match.
> 
> My general point is that the UI code, after migrating it to GTK+ 3, has
> some parts that could benefit from some refactoring, e.g. removing
> certain now unnecessary functions (especially GtkAction/GAction related
> stuff). Whether it is rewritten in Vala or in C++ doesn't really matter
> from that perspective.
> 
> Thinking about it again, a rewrite in C++ obviously gives the
> additional benefit of better integrating with the existing core GnuCash
> code.
> 
>> 
>> * Agreed that rewriting the old register isn't attractive, but a
>> developer spent a year trying to get a new GtkTreeView-based register
>> to work right and although he got close, it wasn't suitable for
>> release. I presume that your Gtk3 port is based on that "Register2"
>> code, and since you say that your work isn't releasable either that
>> doesn't augur well for that line of development.
> 
> Yes, my code is entirely based on Register2. As you said it, it still
> has some quirks, but I am sure they can be solved with not too much
> further work. While it is possible to rewrite the old register using
> GooCanvas/Clutter/the GOffice GocCanvas/whatever, it seems like double
> work, since Register2 is quite close.
> 
> When I say that my code is not releasable right now, I do not mean that
> there are fundamental flaws with it - it just needs some more
> polishing. That doesn't mean that my work is fundamentally flawed :D
> 
>> 
>> * The dynamic module system is indeed a complication of dubious
>> utility, but Guile works by dlopening libraries; one doesn't
>> dynamically link them. Having GnuCash's executable statically linked
>> and Guile using dlopened shared libraries won't work: They pass
>> objects back-and-forth between each other so they need to use the
>> same binary image. The Gnucash executable might be able to
>> dynamically link the shared libraries instead of dlopening them on
>> startup. It's worth trying out. We do have a long-term goal of
>> getting Guile out of the middle of GnuCash and that would make static
>> linking more feasible.  
> 
> Thanks for the clarification, that's what I expected.
> 
>> 
>> *We (I) did extensive performance testing with several multi-
>> precision libraries including boost.multiprecision and found that
>> their dependence on the free store made them unacceptably slow.
>> GncInt128 uses the same algorithms (they're in Knuth) but is stack
>> based and so substantially faster than boost.multiprecision. The gory
>> details can be found by following the long thread beginning at https:
>> //lists.gnucash.org/pipermail/gnucash-devel/2014-May/037723.html.
> 
> Again, thank you very much for the clarification. That thread was an
> interesting read.
> 
>> * We certainly recognize that GnuCash's GUI design is a bit dated,
>> but a redesign depends to a large degree on the toolkit one uses.
>> Since we're not ready to choose a new toolkit we're also not ready to
>> undertake a major redesign. Regardless, the Gnome3 HIG is unlikely to
>> factor much in our design decisions.
> 
> You mentioned above that GTK+ 3 is not the direction you want to take
> the project, and you suggest that the design direction you want to take
> is should be based on the "new" toolkit.
> To be honest, I don't think GnuCash needs to have one exclusive and
> authorative UI, and I don't really see a problem supporting more than
> only one toolkit - just like GnuCash has a SQLite, a MySQL and an XML
> backend and supports Windows, Mac OS X and Linux. In fact, other
> projects like VLC have done that for years.
> 
> Now, let's say that some developers would like to include a Qt UI.
> CuteCash obviously already succeeded doing that, even if it has not
> received significant improvements since 2011 and is based on the
> somewhat outdated Qt4 toolkit. Is there any reason not to have both
> GnuCash (if GnuCash will be the GTK GnuCash) and CuteCash? I wouldn't
> say so.
> 
> Since I like GTK+, I would prefer to have GnuCash use, besides any
> other toolkits, GTK+ 3, where new development happens - and I see no
> reason not to change GnuCash to support GTK+ 3 and to move the UI a bit
> close to the Gnome 3 HIG. Please bear in mind that this does not
> necessitate a complete and major redesign of the entire UI.
> 
> All in all, thanks again for your feedback and I'm looking forward to
> your reply!

Geert's done a good job of explaining some of the problems with your commits. I'd like to add to that that I've been down the road of trying to rearrange a too-random commit series into something rational and failed. It's not enough that each commit implement a single conceptual change as described in its commit message: It's also necessary that each commit compiles and passes the tests. If that breaks down then git bisect becomes useless for debugging regressions and that's too important a tool to allow that to happen.

An example of an extraneous change that doesn't belong in this branch—and shouldn't be made regardless—is fef6acc, "Change unusual abbreviation in AUTHORS". "&c" is unusual in the US, but common in the UK. It's not obvious in most typewriter fonts, but an ampersand is actually a rather distorted Et, latin for "and".

1b80944, one of the "Gnome WIP" commits, seems to mix some extract-function refactors, some GApplication implementation, gratuitous edits (e.g. replacing the string "Gnucash Preferences" with "window preferences"), gratuitous commenting-out of debug code and code modernization (e.g. replacing gtk_alignment_set_padding with gtk_widget_set_margin_start), and a complete rewrite of exactly one builder file encompassing 16 files in total. Teasing that one commit apart so that it makes sense and so that each piece builds and tests will be difficult. You seem to have lots of changes like that.

One of the major problems with both the register and register2 implementations is that there's a lot of "business logic" code in register that Robert Fewell pasted wholesale into register2. That code needs to be extracted to a (single) separate layer so that the GUI code only does GUI. This problem isn't limited to register, either: There's tons of "business logic" code spattered throughout gnome and gnome-utils. Had you come to us first I would have explained that to you and tried to get you to work on that first. I'd still like you to do that (in its own feature branch) if you're willing.

If you haven't already done so you should look at all of the Register2 bugs in bugzilla. Register2 has its own category to make that easy.

Since you're presumably familiar with Gtk+ coding style, GnuCash's is the same with one exception: We indent 4 spaces instead of 2.

I'm not in favor of Vala even for Gtk development because very few developers know it. Guile has been an enormous barrier to recruitment for exactly the same reason, and I don't want to compound that problem.

Cutecash is a demo. It implements only part of GnuCash and while Christian tidies things up periodically to keep it working it has never become a serious alternative to Gtk. I think that that's because of the MVC violations I mentioned earlier, but only Christian really knows. When Christian wrote it we were still using subversion and it wasn't feasible for him to put it anywhere but in the main tree. Now with Git he'd probably have it on his personal Github repo.

Regards,
John Ralls