Bringing CherryTree to C++

[txe]

Hi @giuspen
As my first step, I consider porting menus.py. It looks like a good task to start, so I can begin working on it if you don't mind.

[giuspen]

Hi @txe, that is perfect but please note that the gtk ui manager used in pygtk2 is now deprecated, so the porting should not be exactly one-one, please have a look at https://developer.gnome.org/gtkmm-tutorial/stable/chapter-menus-and-toolbars.html.en and generally at https://developer.gnome.org/gtkmm-tutorial/stable/index.html.en for examples /recommended techniques. Furthermore if you download the latest gtkmm3 source code you can build the demo code with "make check". Cheers!

[txe]

@giuspen
I've made a menu using gtkmm approach, but there's no way to add menu icons through Gtk::Builder or Gio::Menu (looks like they miss this functionality because menu icons are not recommended any longer under the GNOME HIG)
I checked other Gtk3 projects and it looks like they either use deprecated Gtk::UIManager or generate GtkMenu manually.
I'd like to see icons in menu, so maybe I will try to the approach with GtkMenu? You can see how it's done in inkscape

[giuspen]

@txe agree with you on the icons, if this is possible only via non deprecated GtkMenu let's go for it

[txe]

Hi @giuspen, after #446 and #447 can you point out what is better to do the next?

[giuspen]

Hi @txe it depends if you want to keep it simple for now or delve into difficult stuff. If you want to do something simple but time consuming that would be the preferences dialog or also the various right click menus. An example of what could be done straight away is the export to txt and html since all the content is there. Or the right click menus. Or you rather start to manage editing the contents? It's up to what you would like to work mostly

[txe]

Hmm, to make it consistent, I'll take popup menus (done: #448)

[txe]

I'm going to work on the preferences dialog (done: #451)

[txe]

Hi @giuspen
While I worked on porting code, I had to read 'legacy' python code, new C++ code and also code from other projects, e.g. inkscape. And I found out that CamelCase code is not readable as should be. While I could easy scan through python code with underscores, I had to spend time just to 'parse' CamelCase function/variable names and longer the name (more words) harder to read.

Why did you decide to use other code style in the new project? Could you consider returning to the old code style or some other style like in inscape (although, they are not always consistent)

[giuspen]

Hi @txe, the reason for camel case is that it is what I'm used to in my company. In fact all the style is nicked from my company. I like that it's nicely differentiating the non gtkmm library methods against the methods belonging to the library. Be careful about inkscape because that was a C/GTK project, only subsequently gradually migrated to Gtkmm. In fact I consider the underscores naming more C style. On the other hand I'm not against using the underscores; especially if this is very important to you. More important to me is the great work that you are doing in the porting so I'm happy to change route in that (except for the class names). You are free to rename what you wish adopting the underscores.

[txe]

Big thanks, @giuspen! I really appreciate it and will try not to abuse your goodwill. About the next steps, I'm going to port some smaller functionality, the last one was rather huge and it wore me out a bit. Your advice will be welcome!

[txe]

BTW @giuspen, I wonder how you see the project architecture, to be more precise, where better keep code related to 'actions'. I thought about keeping it in cp_app/ct_main_win/ct_treestore/etc, but it looks like this will make these classes too mess, so I got another idea:

• keep general\helper functions and signal handlers in CtApp, CtMainWin, CtTreeStore,
• keep actions in special action classes, for example, CtEditorAction, CtTreeAction and etc., while object of these classes will get references to CtApp, CtMainMain and will be able to use them.

class CtApp {
private:
    CtTreeAction _ctTreeAction;

    void somewhere() {
           _ctTreeAction.init(this, main_win(), _treestore, something_else);
    }
}

class CtTreeAction {
     void add_node();
     void delete_node();
     // etc
}

What do you think? It doesn't sound too odd? Also, maybe you get some ideas you can share?

[giuspen]

@txe I agree it is tidier to keep the actions separate. In the past it happened me already to have separate files for the callbacks. But it may be sufficient to have the body of the actions in separate dedicated .cc files pointing to the same header class and group the actions nicely in the class header. This also applies generally if a class .cc files gets too big to split into logical source files. My view is to avoid the spreading of too many classes layers where not strictly necessary, but anyway if you want to try your approach on part of the code I will give you feedback later and we can rework if necessary. About the next step I had in mind was to allow the links to work if you want to give it a try but if you have other preferences feel free. I will try to get back on the gtkmm code in the early mornings of the next week (uk time)

[txe]

OK, I see your point although it is quite unusual to have separate .cc. Can you come up with a suitable name for the class, nothing good comes to my mind? And then, it will be like like ct_classname.h, ct_classname_editor.cc, ct_classname_tree.cc and etc. (so they will be nicely sorted in a IDE project tree)?
The next thing I want to do is adding/delete/duplicate nodes if the current codebase allows it.

[giuspen]

Exactly, same prefix of the header and an additional specification just like your example. You can start with the node add/delete/duplicate that should be fine, and maybe I'll start to work on the save, save as and export to Cherrytree document

[txe]

Hi @giuspen what do you think about using fmt in the project? it's handy and fast and never failed me.

[giuspen]

Hi @txe seems fine to me I'm just wondering if it will be working fine with the utf-8 Glib::ustring and i18n

[txe]

Hi @giuspen
I'm not sure if commands from Edit\Formatting\Search do not depend on some missing sourceview features or another core stuff and thus cannot be implemented.
What is the status of the source view, can I do something for it?

[giuspen]

Hi @txe, the visualisation is all there already so you can surely start working on the search. About the replace and generally the edit, that can still be done even if the undo/redo state machine is not there and neither the possibility to save the modifications to disk.

I raised an important question at https://stackoverflow.com/questions/54529359/gtkmm-3-text-view-anchored-in-text-view-cannot-get-cursor-inside and also wrote to the gtk mailing list but got nothing, this is a critical thing if anyone knows of a channel where that could be advertised please go for it.

[txe]

It looks like C++11 regex doesn't support a multiline option (it has been supported since C++17). I think it's OK to keep the built-in regex for now, and later we can switch to something else like PCRE.

[giuspen]

I think we can go C++17, we are currently C++14 but I was already thinking to go 17 in case any of the new features would have been useful to us

[txe]

Sure, then I'll switch to C++17

[txe]

Hi @giuspen, it looks like there are several issues with regex although I likely have a solution to them:

1. std::regex_search doesn't have start_from(offset) input parameter to start regex not from the string beginning. Using substring or first&last input iterators is not easy because of offset converting. I fixed it by using Glib::Regex that can do everything we need and actually looks more mature than std::regex.
2. TextBuffer works with symbols and uses positions in symbols, regex works with bytes and returns positions in bytes. So, TextBuffer returns a utf-8 string where symbols can be both one or two bytes and, actually, Glib::Regex can use this string without problem. The problem is that it's not easy to convert a byte index into a symbol index and vice versa (OMG, why they cannot always use two bytes per symbol?). As I see Glib::ustring doesn't have means for the converting, so we have to parse the string by hand just to find out these indexes.

I checked other Gtk projects for the solution and maybe GtkSourceSearchContext resolves all problems (although it is not in gtkmm). What do you think?

[giuspen]

A little one @txe that I found yesterday, since I would be tempted to rework it a bit to fix the issue, I prefer instead to have your feedback on it. Basically if you open a password protected document and then you change your mind and press cancel on the dialog asking for the password, there is an error dialog saying that there was an error parsing the document, this is incorrect. I looked at the code and I see that the user pressing cancel is managed with an exception, I don't particularly like it because also for example if the last document you opened was a password protected one, then the day after you start cherrytree and it asks you for the password for that document but you want another document now so you press cancel, that is a typical use case, not an exceptional use case.
I would prefer to manage returned values rather than exceptions with an error in cases that are not an error but a normal use case but let me know your point. BTW I'm planning to package a new 0.99.x in the evening with the new super duper export to pdf with links ( thanks again @ForeverRainbow )!

[txe]

Yeah, I remember this issue and I had the same thoughts, only one thing stopped me, it was annoyingly uneasy to pass the fact that user canceled the dialog. Maybe throwing a custom exception for that and then, as a result, not showing any messages, is a bit better. But anyway if you have in mind how to fix it, go ahead.

[giuspen]

I thought as an easy at least temporary workaround to parse the error message but that would be a bit flaky because then if you change the message we're back. I'll have a thought anyway, I may push something later.

[txe]

Links are really cool. Btw, how is your progress with testing? Did you do as much as you wanted?

[giuspen]

I made a good progress with my read and write unit tests but I still have to add the embedded widgets harness, hopefully that will come in the next few days. After that it should be fairly easy also to set up unit tests on the imports from other applications into cherrytree when gradually implementing that.

[ForeverRainbow]

Why not make it return an std::optional instead of throwing, or split it into two functions one which gets the password (which is empty if cancelled) and one which decrypts it (which would imho be cleaner).

I can implement that now actually if it sounds good

[giuspen]

Hi @ForeverRainbow I actually already implemented something, currently in 0.99.4, that solves the issue. I returned an empty extracted filepath in case of user cancel rather than throw an error and up the stack in case of empty error I'm not showing the error dialog.

[giuspen]

Hi @txe at the time of implementing the read/write of the tables, I thought it was the right time to stop moving the header to the end (which I have kept doing for backwards compatibility for the pygtk2 life) so I implemented this additional xml attribute "head_front" that, if found at reading time, would mean that there was no need to move the last line to the first position since the rows were already in the right order. I think that I was already writing this argument to true at the time of writing the table but now I cannot find that and in fact the check bool head_back = xml_element->get_attribute_value("head_front").empty() is always false. This would indeed cause an issue for who jumps between the two versions so I should support it in the pygtk2 version as well before re-enforcing it in Gtkmm3 but before I wanted to get what you think about it, do you think it is the right thing to do or you would rather stick with the current format and drop the "head_front"?

[txe]

Sorry about it, I didn't mention that change. I was freaked out when I opened files in pygtk and all tables were broken. But you see, no complains about tables now :)

I don't know, I don't have strong arguments for both cases.
If it was up to me, I would just play it safe and leave it as is because though it definitely looks odd, it does no harm. And having two slightly different formats, checking it every time and maybe having complaining users are bit messy.

But I would certainly add version checking in pygtk because pygtk can read new format (when some changes will be introduced), lose some information while reading and save it wrongly, especially when it's xml.

[giuspen]

That's fine, the plan was to update the Pygtk2 version to support that attribute before the Gtkmm3 version would go public but I've never looked at that until today when I was looking at unit testing the tables and I remembered I didn't want to see the header at the bottom anymore... but there it was :)
I'll leave it as it is for now, other issues have definitely higher priority

[ForeverRainbow]

I am wondering whether it would be better to split the import parsers into a separate application, run from ct_imports. It would make it easier to unit test and provide a language independent interface for converting to cherrytree XML . I am thinking it might work something like:

Most things in ct_parser are moved into a separate binary which is built along with cherrytree. The things in ct_imports then run the separate binary with formatting args and parse the output, turning it into a tree of ct_imported_node's so the import interface won't change.

The separate binary takes formatting args, reads from stdin and outputs some simple container XML, e.g <file><root><slot><rich_text>...</file><file>... which just retains the hierarchy and the converted rich text.

What can't be moved is the markdown real-time formatter and since it is quite heavily connected to the markdown parser that may be tricky to separate.

I like this idea because it would allow changes and fixes to the parser implementations separate from the main source, and would also allow external scripts to utilise the parsers. It may be not such a good idea though, any thoughts?

[giuspen]

I thought about this when I wanted to reuse the python code to import. If I would not have had your help I probably would have opted for an intermediate conversion to cherrytree file document with python and then importing the cherrytree document into the (cherry) tree. At this point I'm not sure, frankly I haven't looked much at the import part. Unit testing is feasible already on the tree after importing the data as I pointed you in the other issue. @txe what do you think?

[giuspen]

We could also use the python (converted to python3) for the remaining documents to import until and if we decide to convert to C++

[ForeverRainbow]

@giuspen The python importer is quite heavily connected to the rest of cherrytree I think? The C++ version only uses CtConfig and only for knowing what type of bullet points to use. I wonder if it would be easier to convert it to python3 and separate it from the rest or to implement the missing importers in C++. But then python has things like BeautifulSoup

[giuspen]

@ForeverRainbow I have no doubt that ideally we don't want that python, it is just a workaround since the time it takes to do all of them is a lot, considering that the inefficiency of the importer for few proprietary types (and often discontinued) does not have any impact on the normal cherrytree efficiency

[txe]

I don't think that separating import code into a different application is improvement:

• current import classes can be already unit tested, they have a simple interface: input - paths; output - xml. If it is not much, it can be improved.
• an additional app can give troubles in distribution
• even to run and fix code is much simpler then it is one project

[ForeverRainbow]

@txe It would still be in one project, and to "run and fix code" would be simpler since you could test the parsers with custom input data separately from everything else. The importer interface would remain the same, they would just call the binary to do the parsing instead of instancing classes. Troubles in distribution yes but if built as part of the same CMake system then it would be like the other dependencies included into the source.

Your call, just seems like an opportunity to make cherrytree more modular

[txe]

I think we can skip an additional abstraction layer if it doesn't resolve any real-world issue.

[ForeverRainbow]

@giuspen I did want to say, I noticed you use alien to generate the rpm's, I haven't used alien but I am on fedora so I can generate them natively if that helps (I locally build each release rpm and install it manually anyway :p).

[giuspen]

@ForeverRainbow thanks I will keep it in mind, the thing is in fedora there is not an LTS strategy so there would be multiple versions to cover am I right?

[ForeverRainbow]

@giuspen I could only build packages for x86 64bit and whatever the latest version of fedora is (currently 32), but it would probably be better than whatever alien produces. Fedora only provides maintenance for ~13 months so yeah no LTS but also no need to package for systems older than that time frame (since people will not be using fedora unless they are preprepared to update to the new releases).

[giuspen]

@ForeverRainbow thanks I will write you an email when I issue a new version so you can package on fedora for me. I did not plan to use alien for the C++ version anyway because the dependencies are much more important than with the python version so hardly a package for a distro is good for another.

[ForeverRainbow]

Maybe not do this now but these are my thoughts on file logging, it (should) be mostly trivial to implement:

I think there should be an option for this though since writing a few mb of debug logs to every time a large export/import happens is probably not ideal :p. That is quite easy to configure since spdlog has its log level set at runtime and it can be changed dynamically (and for different loggers so terminal could do trace and the file do info, for example, or separate error and general logs, etc).

I tried to find something like QStandardPaths for Gtk but I couldn't, does anyone know if such a thing exists outside Qt? If not I can add wrappers in fs which return the log paths for each system (would prefer not to do that manually though).

[giuspen]

I'm not understanding, why would we write MB of logs for imports/exports other than when debugging it

[ForeverRainbow]

@giuspen That was mostly a joke, it is not really mb but it is a considerable amount, when logging at debug level exporting at few thousand page pdf will print over 10k lines or so. If that accumulates it will start to add up and its an extra overhead which isn't needed 99% of the time

[chadfurman]

Is there a current TODO set on this project?

[chadfurman]

Perhaps #828 -- but it's closed. Is the C++ port complete?

[giuspen]

The TODOs are the Github issues. The porting is complete except for some imports from other note taking apps still in the queue.