GNOME Bugzilla – Bug 764441
enhancement: Implement language switching feature.
Last modified: 2017-12-21 16:49:12 UTC
hotdoc (https://github.com/hotdoc/hotdoc) can generate documentation for different programming languages for introspected GObject libraries. After discussing with Frédéric, being able to switch from a language to another natively in devhelp seems to be a desired feature. The proposed solution is as follows: 1. I had incorrectly told Frédéric devhelp accepted multiple books with the same names, it was incorrect, I mixed up title and name attributes. Devhelp should accept multiple books with the same name, that way we can generate one book per language. This bug separates the notion of book id from the notion of book name, fixing in the process a longstanding issue in dh-link. In the current state books with multiple names can now coexist, uniqueness is enforced on book ids, which are implemented as a composite of book names and book language. Commit messages explain changes in more details. 2. In a future bug, I will implement the model-side aspect of language switching, after discovering the available books, devhelp will perform an extra step, disabling duplicate book names, only keeping the preferred default language enabled. 3. The last phase will be the UI side of things, where devhelp will need to expose a language switching combobox when needed, which will navigate to the correct book when activated. I've put up a branch on github to ease testing at https://github.com/hotdoc/devhelp/commits/language-switching Cheers!
Created attachment 325120 [details] [review] dh-util: define some useful logging macros. This to make sure devhelp logging is kept under the devhelp namespace. + Port the only place where g_debug was used.
Created attachment 325121 [details] [review] link, parser, book: Prepare language switching. Make links hold a pointer to the book they belong in. As it is assumed links always belong to the book's keyword list, no reference is held to avoid circular dependencies. Books now hold their "base", which fixes a longstanding FIXME in link. The goal with this is to implement in the next commit the notion of book id, function of its name and other parameters, in order to allow for multiple books with the same name but for example different languages, or potentially in the future different versions. Ideally, we could simplify the dh_parser_read_file method in a follow-up commit to only accept the book, as its current prototype is a bit weird, this implies exposing setters in book, which I didn't want to do to keep the commit easily reviewable.
Created attachment 325122 [details] [review] link: add missing G_BEGIN_DECLS / G_END_DECLS Just a drive-by patch
Created attachment 325123 [details] [review] book: Construct unique id. The id is a composite of the name and the language. This can be extended the future to handle version numbers as well, if we really want to go beyond that afterwards it might make sense to refactor devhelp in order to store books in an actual database, this isn't worth the added complexity for now.
Created attachment 325124 [details] [review] book, manager, model: Lift requirement for unique name. We now enforce uniqueness based on the id implemented in previous patches. Multiple books with the same name can now coexist, a debatable effect of this patch is that filtering by books now requires the user to suffix the book name with the language of the documentation. Apart from that, no obvious regressions from manual testing, disabling and enabling books works when multiple books with the same name exist.
Created attachment 325199 [details] [review] book: add FIXME for language string. book_get_language should really return the actual language, without a "Language:" prefix.
Created attachment 325200 [details] [review] book-manager: implement filtering of same-name books. Priority is given to books with the C language when multiple versions of a book with a different language exist.
Created attachment 325201 [details] [review] link: add "book" getter
Created attachment 325202 [details] [review] window: Implement language-switching. This adds a combobox to the window, visible when multiple versions of same-named books exist. When switching from one book to another, the old book is disabled and the new one, unsurprisingly, enabled. The code tries to find an equivalent page to redirect the user to, so that they can switch between the documentation of the same symbol in different languages in one click. When no matching page is found, the user gets redirected to the root page of the new book version, some sort of warning should be displayed in that case, up for discussion.
I ended up implementing phases 2 and 3 before phase 1 was merged, I'll use this bug for the whole work and rename it.
Ping ?
book-manager: implement filtering of same-name books. + /* For now we just want C to be preferred */ + /* This Language: prefix is very hackish, there's a FIXME + * in dh-book for that already */ + if (!g_strcmp0(language_a, "Language: C") && + g_strcmp0(language_b, "Language: C")) { + return -1; Indeed, this won't even work as dh-book will put a translated string in there. What about adding a new language_code to DhBook, that would hold the unmodified value, straight from the XML attribute? > When no matching page is found, the user gets redirected to > the root page of the new book version, some sort of warning > should be displayed in that case, up for discussion. I think it's fine to redirect to the homepage, without displaying any sort of notice.
Great, I take it you're OK with the rest of the patches then ? I'll look at the language issue see if I think of a correct way of doing this instead of a lazy FIXME, shouldn't be too tough, ideally we wouldn't have both language and language_code, seems a bit messy :) I'm in holidays right now, will have more serious things to say in a week.
Review of attachment 325200 [details] [review]: Obsoleted by https://bugzilla.gnome.org/show_bug.cgi?id=771327 , rejecting
Review of attachment 325202 [details] [review]: Obsoleted by https://bugzilla.gnome.org/show_bug.cgi?id=771327, rejecting
After discussion, we figured it would be best to enable javascript, and not duplicate logic in devhelp, a few of these patches I've marked as rejected for that reason, however some others would be worth merging IMHO, this is not pressing in any way but might as well merge them if it's OK before they bit-rot :)
For reference: https://bugzilla.gnome.org/show_bug.cgi?id=771327
So if I understand correctly the language switching feature is implemented in JavaScript by HotDoc. HotDoc provides one book (per library), and in that book it contains the documentation for different programming languages. It anyway makes sense to enable JavaScript, regardless of this feature, because JavaScript could be used for other purposes in API documentation. I've looked at the non-rejected patches, they are all obsolete now, they probably contain a lot of conflicts with the current code. I've done a lot of code cleanups in dh-parser, DhLink and DhBook in 2017. I've fixed the problem with misleading function names about book name/id/title. So I close the bug. Maybe implementing language switching in Devhelp will come again in the future if one day GTK-Doc can write documentation for different programming languages (or another API doc generation tool).