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

[Tobias Markus]

On Mo, 2016-02-22 at 22:57 +0100, Geert Janssens wrote:
> Hi Tobias,
>  
> For some reason I missed your original announcement. Your reply to
> John made me find it.
>  
> Anyway...

Hi Geert,

thanks for your feedback, I appreciate it.

>  
> First off, thanks for the effort you have spent so far to migrate
> gnucash to gtk+3. I gather this is a huge job (and the size of your
> branch confirms this).
>  
> I agree with John we haven't decided on our future toolkit yet.
> However I also believe gtk+2 is a dead end and porting from gtk+2 to
> gtk+3 is currently the least effort we can make to stay relevant.
> Even if it may turn out gtk+3 won't be the future officially endorsed
> toolkit by the core developers at some point, it will bring gnucash
> now to a actively supported toolkit. So I am all for getting gnucash
> running under gtk+3.

Thanks a lot.
>  
> Having said that, I unfortunately also have a few reservations with
> your current approach.
>  
> I looked a a few commits in your branch. My first impression is that
> the conversion happened slightly chaotically (like in a coding
> frenzy). That's fine for a first code run. However we prefer our
> central code to be more structured. The idea is that not only you at
> this point in time needs to understand what happened. All other
> gnucash developers should as well. And that means both at present as
> well as 5 years in the future.
>  
> So to get your work merged it will need some clean up at least.

That is totally understandable.

>  
> A few general suggestions:
> 1. Better commit messages. Each commit message should summarize what
> the commit does.
> 2. Make changes as atomic as possible. One commit should focus on one
> change. Depending on the change, a commit can be large or small. As
> long as it remains clear what that commit is doing.
> 3. Split out cosmetic changes (like whitespace and indentation fixes)
> from functional changes.

That's what I have in mind - it's just a lot of work for the ~150
commits I already have, not to mention those to come :D

>  
> I'll add more comments below between your message.
>  
> On Sunday 21 February 2016 20:12:58 Tobias Markus 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.
>  
> No need to apologize and we're not in a hurry. As said before
> however, your branch will need tidying before it can be merged.
>  
> > 
> > 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.
> > 
>  
> Well, current release is obviously the baseline to compare with. If
> there are regressions in the current functionality, these need to be
> fixed indeed before release.
>  
> > Because GnuCash 2.0 was (afaik) coupled to the GTK+ 2 migration, I
> > suggest that the GTK+ 3 version should be GnuCash 3.0.
> > 
>  
> Perhaps indeed.
>  
> > 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.
> > 
> We will have to check what's available in the distributions we intend
> to support. The two most conservative ones typically are Debian and
> RHEL.
> Current stable Debian (8.0/Jesse) is at gtk+ 3.14.5. I don't think
> Debian will have another major release at least one year before the
> next major gnucash release.
> RHEL (7.2) is at gtk+ 3.14.13.
>  
> So I expect we will need to support 3.14.
That should be doable.

I'm really not a distributions expert, but wouldn't Debian for example
wait anyway before picking up a version 3.0.0? I thought they would
probably stay with the latest stable series anyway.
>  
> > 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.
>  
> What's the motivation to remove the splash screen ? GnuCash is a slow
> loading application. Several users prefer to have a splash screen
> indicating progress (like LibreOffice does for example).
> And yes, I understand that we could optimize loading speed to avoid
> the need for a splash screen altogether. But we're not there yet by a
> long shot.
This is more of a temporary measure. I reasoned that if the loading
speed was improved significantly in time for the release, then the
splash screen wouldn't be needed anymore. And if startup speed was
still rather slow, reintegrating the splash screen into the
GApplication-based code would need some thought anyway to make sure it
integrates well.

GApplication provides a way to indicate to the user that the
application is currently busy via g_application_mark_busy. I included
that in the startup process, so even if the splash screen wasn't needed
anymore, the user would receive some feedback.
>  
> > * I updated the HTML code to use the Webkit2 API, but it needs some
> >   more love.
>  
> That's a nice improvement. We were lagging on the Webkit upgrades.
> There is an important reason for that though: it's almost impossible
> to build webkit on Windows. Do you intend to tackle that part as well
> ? We'll need a usable Webkit2 library on Windows before we can land
> your webkit2 api.
Sorry, but I'm no expert in doing Unix-ish stuff on Windows :D
>  
> As an aside: I would prefer to see the migration to the Webkit2 API
> as a separate project from the gtk3 migration to reduce the amount of
> moving parts per branch. If gtk3 migration depends on webkit2 API, it
> should be done first. If not, it can also be done after the gtk3
> migration has stabilized.
Please have a look at my answer to John's post, but in short: Porting
the code to WebKit2 was a breeze, but I haven't conducted any thorough
testing yet.
>  
> > * Using GtkApplication renders everything related to
> >   gtk-osx-integration unnecessary.
>  
> That would certainly be nice. John is in a better position to confirm
> or deny this.
>  
> > * This is unrelated to the GTK+ 3 migration, but I added a Markdown
> >   version of the README and added a DOAP project description.
>  
> So now there are 2 README's to keep in sync and an additional DOAP
> description. I would propose to keep only one README file. I have no
> strong preference over plain vs Markdown. What benefit is the
> Markdown edition ?

Markdown is a standardized markup format, meaning it looks nice in
GitHub and the likes of it. It is also quite human readable, so it
could replace the plain text README.

Note that the Markdown README needs to be updated to include
descriptions for the CMake build process.
>  
> And what benefit does the DOAP file bring us ?

There is no killer feature I can point at right now. DOAP files are
essentially standardized project descriptions and there are various
applications making use of this. I see no reason not to include it -
it's just a single file and does not require any more effort on the
GnuCash side.

>  
> Lastly as you say yourself, this is not Gtk+3 migration related, so
> would best be submitted as a separate PR.

It is on a separate branch, next. (gtk3 is based on next)

>  
> > * I changed the action naming pattern from CamelCaseAction to
> >   dot.separated.action-name, e.g. file.close or account.d
> > 
> Again what is the motivation for this ?

Consistency: The GAction infrastructure introduces dot-separated
prefixes. The most import of those are the app. and win. prefixes found
in the GMenu descriptions, which indicate whether an action is
application-wide or window-specific. (It seems much better to call an
action win.file.import.csv than win.FileImportImportCsvAction.) There
are some other ways to make use of custom prefixes which I didn't
include yet.

>  
> > 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.
>  
> Yep.
>  
> > * AqBanking uses the gwenhywfar UI library, which in turn uses GTK+
> 2.
> > I have not yet looked into this.
>  
> Ok. Just stating the obvious: this may require patches to gwenhywfar
> upstream.
>  
> > * I removed the old file history/recent files code. There are
> better
> >   ways to do this in GTK+ 3.
>  
> Such as ?

I haven't looked into this in detail yet, but it seems that recent
files are supported by the file chooser dialog without any requirements
in the application code. Try File->Open, the sidebar includes an
element called "Recent".

>  
> > * I haven't converted every UI definition file to the new format
> for
> >   now.
>  
> Ok.
>  
> > 
> > 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.
>  
> As John said, we will likely drop autotools at some point. Cmake is a
> very recent addition so it needs some time to mature.
>  
> > * 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.)
>  
> I looked into rewriting the old register at some point. It's
> certainly viable. I agree it not being very useful though.
>  
> Of course we intend to ship only one register. During development
> though it's convenient to be able to use both registers side by side.
>  
> The main advantage of such a set up is that you can quickly compare
> behavior for both in the same gnucash run. We had several brave users
> helping out in the beta testing which didn't have sufficient
> experience to rebuild gnucash. I would prefer to keep this situation
> until we're sure the new register is matching or superseding the old
> one stability and feature wise.

Unfortunately, the old register just won't compile right now, since
Gnome Canvas has been dropped completely. It certainly is possible to
port it to GooCanvas, but I think there are better ways to spend your
time.

Aside from porting it to GooCanvas, you would ideally also migrate it
to use style sheet based rendering (essentially everything that
modifies colors would need some work).

>  
> > * 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.
>  
> There is. You can run gnucash in headless mode to update price
> quotes. The headless mode is not using all the modules and wouldn't
> want to. I am not really in favor of this construct for several
> reasons. However that's the current situation. I think a nice way out
> of this is creating a separate binary specifically for the price
> quotes updates.

For simplicity, I temporarily dropped the fetch price quotes feature
because it wouldn't have been viable to include it in the new
GApplication code. Creating a separate binary, as you said it, should
be rather simple and get the job done. That was my thought when I
implemented the move to GApplication.

>  
> >   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.)
> > 
> John already suggested guile requires dynamic linking. And there's
> not only guile, we also more or less support python bindings.
>  
> I do agree that GModule is most likely overkill. Converting away from
> that needs to be done carefully. Most of our gmodules perform one-
> time initialization steps that will need to find another place.
>  
> And care must be taken this is called from all possible code paths
> (main gnucash application, guile bindings, python bindings, quotes
> updater,...)
>  
> And here also, this work is unrelated to gtk3 migration and can be
> done in a separate branch.

Yes.

>  
>  
> > 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 don't know vala (though I have heard of it). And I have heard both
> vala enthousiasts as well as vala haters (even Gnome Builder's
> Christian Hergert seems to be in the latter camp). Considering where
> the current coding efforts are headed, vala can have a place in the
> gui code at best. The core code will be in c++. And then my next
> question is (again) portability. Is vala readily available on OS X
> and Windows (more precisely in mingw) ? If not, it would be a non-
> starter unfortunately.

Yeah, gtkmm/C++ really seems like a better idea.

>  
> > * 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.
>  
> Nice at first sight and from a coding point of view. I'm not
> convinced yet it will also improve the user experience.
>  
> Unlike a text to a word processor, gnucash' data is not linear. For
> example transactions (or more precisely splits) are organized into
> accounts which are represented in account registers. There isn't one
> single register in which you can edit *all* splits. Well, there is of
> course (the general journal), but it's most difficult interface to
> use to manipulate and interpret your data so registers per account
> were implemented.
> Then there are reports of all kinds, interfaces for invoices/bills,
> budgets, scheduled transactions,...
> Each of them has it's own user interface.
> Having only one window would mean only one interface can be visible
> at a time. To quickly switch between interfaces tabs could be used.
> Oh, wait, this is the default behavior of gnucash. Then suppose I
> want to compare the contents of my bank account register to the
> report I ran on it. Or I want to compare my credit card register with
> my bank account register. These are real life situations. I can't do
> this with one window with tabs. And I know users who definitely
> prefer each tab to be in a separate window instead for this very
> reason.

I agree. I think this area might benefit from some further thought.

>  
> >   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.
>  
> I understand your reasoning. On the other hand the current gnucash
> concept is that you can have only one file open at once. So all
> windows belong to that file. So gnucash users do know that all
> windows change the opened file. It's a different paradigm from
> typical applications. It's worse even, gnucash currently *can't* have
> multiple files open at once. The internal code just can't handle
> that. (To be complete there is a hacky work around by opening multipe
> instances of gnucash itself, but that's not relevant here).
>  
> I personally find this to be a design oversight from the early days.
> But again that's the legacy we have now.
>  
> Having said all that, if we can come up with an elegant UX within a
> single window interface allowing the interactions currently possible
> with the multi-window interface, I wouldn't mind going for single-
> window. I don't see that yet though...

On the other hand, why not combine both approaches and offer both
multi-window and multi-file workflows, i.e. 1 GtkApplication to 1 File
and 1 GtkApplication to Many Gtk(Application)Windows?

>  
> > * UI design: see below
> > * I have not yet looked at translations and accelerators.
>  
> Ok.
>  
> > * The new register requires some more love.
> > 
> Did you do anything to the new register other than converting it to
> gtk3 ?
>  
> Also here I would have preferred to see the new register stabilized
> and finished in the gtk2 edition *before* moving it to gtk3. Doing
> the two together gives us 2 moving targets and makes it more
> difficult to track where a future problem exactly started.

I wanted to prevent changing something from one deprecated interface to
another deprecrated interface and only discovering so in the following
migration work.

>  
> > Nice-to-haves regarding GTK:
> > * Some GSimpleActions would be better off as GPropertyActions.
>  
> Probably fine.
>  
> > * Validate all GtkBuilder files using gtk-builder-tool.
>  
> Would be nice indeed.
>  
> > * File Chooser Dialog: Implement proper file format filter.
>  
> What would that look like ? Old gnucash versions didn't set file
> extensions. And the default extension used to be .xac for a long time
> before it was renamed to .gnucash. So you can have a gnucash book
> called
> foo
> foo.xac
> foo.gnucash
>  
> The last two would be easy. However how can you filter files not
> having an extension yet only show the ones that are actually gnucash
> files ?

Well, while we are breaking stuff anyway, why not just tell users to
save their files as .gnucash or whatever? Have a big warning pop up at
startup saying they should make sure their files have the proper
extension? Anyway, they still have the option of viewing all files in
the file chooser dialog.

>  
> > * xdg-app support
>  
> Is that something that should happen from within the gnucash project
> ?

Afaik, it would be part of the build process and thus needs some
integration on the GnuCash side. This really is a nice-to-have "because
all the cool kids do it", so it's not urgent at all.

>  
> > * jqplot as a Git submodule
>  
> Ok.
>  
> > * Have hints for GtkEntries
>  
> Ok.
>  
> > 
> > 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.
>  
> Do you see this as optional or mandatory ? The latter would then
> require all developers to have clang on their system ? What about
> clang on Windows ?

Really, this doesn't have to be mandatory. I think you might be able to
run this on the server as a push hook or something similar, obivating
the need for any client to have Clang installed.

>  
> > 
> > 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.
> > 
> Ok.
>  
> > 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
> > 
> I'm not ready yet for such changes. My general impression is this
> works well for simple applications but quickly falls short for more
> complicated ones. GnuCash doesn't aim to be "so simple your
> grandmother can use it", or tabletized (at least not the desktop
> version). 
>  
> > 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.
>  
> Unfortunately I don't like this trend on desktops. Again my feeling
> is that in most cases the applications are dumbed down to an absolute
> minimum in order to fit the "keep it simple" mantra.
>  
> I am not sure gnucash belongs in that category.

I wouldn't say that KISS is a bad motto: things that are simple should
be done in a simple (as in not-complicated) way, and complex
functionality should be complex, but not complicated. This doesn't
necessarily mean that GnuCash should be dumbed down: It just means that
simple features should be exposed over simple UI interfaces (e.g. a
button with a plus symbol for "New Account"), and more advanced
features don't have to be hidden or removed for that.

All in all, UX is not a theoretical topic, and before we devolve in
long discussions on which mantra is the best, I suggest I do some of
the changes I have in mind and we talk about what the actual prototype
looks like.

>  
> For completeness I will add that I do agree our user experience needs
> an important overhaul. But it's too early for that.
>  
> > 
> > Asking the Gnome Design Team for suggestions is also a possible
> > avenue.
>  
> Absolutely. As would be KDE's VDG if we were to choose Qt as future
> toolkit.
> > 
> > Thanks for your time and attention - let's make GnuCash an even
> better
> > free software finance manager!
> > 
> Thank you for taking on this migration work !
>  
> Regards,
>  
> Geert

Again, thanks for your invaluable feedback!

Best regards,
Tobias