giuspen / cherrytree

cherrytree
https://www.giuspen.net/cherrytree/
Other
3.31k stars 456 forks source link

Bringing CherryTree to C++ #444

Closed txe closed 3 years ago

txe commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

txe commented 5 years ago

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

giuspen commented 5 years ago

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 commented 5 years ago

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

txe commented 5 years ago

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

txe commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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:

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

giuspen commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

txe commented 5 years ago

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?

txe commented 5 years ago

I've ported mostly all find code, so maybe I can write and use converting index functions for now, and someday I'll rewrite code to use GtkSourceSearch

giuspen commented 5 years ago

Hi @txe, I think a gtk solution rather than a gtkmm solution is perfectly acceptable, then it's up to you. Myself raised a bug to https://gitlab.gnome.org/GNOME/gtk/issues/1663 and subscribed to the mailing list but still got no valuable feedback. I will look at the gtk source code myself and try to figure out what from gtk2 to gtk3 broke the functionality. What concerns me is that if indeed it's a bug an there's no workaround we may have to support building against gtk2 for the beginning, what do you think?

txe commented 5 years ago
  1. You may try to rewrite the demo without gtkmm to exclude that gtkmm doesn't make a mess.
  2. I don't know how easy to build gtk libraries from source, so maybe installing older\unstable version of gtk3/2 helps to find out when the bug was introduced.
giuspen commented 5 years ago

Yep, I will try since the very first gtk 3.0 and see if the bug was already there.

txe commented 5 years ago

Hi @giuspen, I took a look at the codebox bug and, in fact, it's quite easy to fix. When you click on textViewNested, it gets focused and actually may show cursor, but the problem is the click event goes further for some reason and gets caught by textViewBase, that thinks it is its click and thus gets focused. So, the different between gtk2.4 & gtk3.0 in propagating the event further or not. The solution is to stop the event like that:

textViewNested.signal_button_press_event().connect([](GdkEventButton* event){
        if (/* left button click*/) return true;
        return false;
});
giuspen commented 5 years ago

Hi @txe thanks so much you must be an angel sent from somewhere

txe commented 5 years ago

FIY, I'm a bit busy later, but I think I can finish formatting actions and lists this week.

giuspen commented 5 years ago

Cool @txe, I'm releasing 0.38.8 in the next hours and subsequently I will work on document write starting from xml

txe commented 5 years ago

Hi @giuspen, I'm working on clipboard.py and can't figure out from where an argument from_codebox comes in copy and cut functions.

giuspen commented 5 years ago

Hi @txe, it is easier to spot that if you run a 'find in files' with the pattern 'clipboard_handler' since 'copy' and 'cut' goes everywhere, in fact not a lucky choice. Doing that you canfind, in core.py self.sourceview.connect("copy-clipboard", self.clipboard_handler.copy, False) self.sourceview.connect("cut-clipboard", self.clipboard_handler.cut, False) And in codeboxes.py anchor.sourceview.connect("copy-clipboard", self.dad.clipboard_handler.copy, True) anchor.sourceview.connect("cut-clipboard", self.dad.clipboard_handler.cut, True)

txe commented 5 years ago

I saw them, but thought True/False meant whether this signal handler should be called before or after the default signal like in gtkmm. Never expect them to be data for a callback. Thanks for help!

txe commented 5 years ago

Hi @giuspen , congratulations for finishing xml document writing! 👍 In case you going to work on the next piece, I'm currently dealing with clipboard.py as well as exports.py, clipboard heavily depends on xml and plain text exports.

giuspen commented 5 years ago

Hi @txe, I was thinking to firstly test the write to xml to actually work as expected and then start with write to sqlite but I'm happy to change plans if you need anything

txe commented 5 years ago

That's perfectly fine! I just wanted to make sure we wouldn't waste time working on the same stuff.

txe commented 5 years ago

Just to be sure, there is a couple of lines in exports.py where pango_font = pango.FontDescription(self.dad.tree_font) It's just some legacy and doing nothing?

giuspen commented 5 years ago

You're right @txe it seems a dead leaf, I saw that in two places and nowhere pango_font is then being used, I will remove those lines if you don't before me

txe commented 5 years ago

At the very end of selection_to_clipboard function:

if not self.force_plain_text:
    targets_vector = [TARGET_CTD_PLAIN_TEXT, TARGETS_HTML[0], TARGETS_HTML[1]]
else:
    targets_vector = [TARGET_CTD_PLAIN_TEXT]
self.clipboard.set_with_data([(t, 0, 0) for t in targets_vector],
                                         self.get_func,
                                         self.clear_func,
                                         (plain_text, None, html_text, pixbuf_target))

pixbuf_target is passed into clipboard, but TARGETS_IMAGES is missing in targets_vector variable. so do I need to add

if pixbuf_target:
    targets_vector.append(TARGETS_IMAGES[0])

or just to remove pixbuf_target (because the text is plain and images will never be there)?

giuspen commented 5 years ago

pixbuf_target should be replaced with None for clarity. the cases of a single character selected are covered before, including the case of a copy image, this is just copy text either rich or plain.

txe commented 5 years ago

Hi @giuspen, It's releated to CtConst strings, I think for the better unicode support we shouldn't use gchar (which is similar to char). Reasons:

On the other hand, using gunichar str[] = "some text"; is not reasonable, the size of unicode character can very from 1 byte to 4 bytes and thus this str is not valid. So, ether we keep using gchar* strings and have to use gutf8 (from gtk) and some custom functions like str::compare(gunichar, gchar) and remember we cannot use gchar strings as ordinary strings or we can use Glib::ustring and never think about this again.

giuspen commented 5 years ago

Hi @txe, I think it makes sense to use Glib::ustring, let's keep it simple

giuspen commented 5 years ago

Hi @txe, I'm having issues to build since you added #include <filesystem> so I temporary applied the changes suggested at https://stackoverflow.com/questions/39231363/fatal-error-filesystem-no-such-file-or-directory but I would rather use http://manual.freeshell.org/glibmm-2.4/reference/html/group__FileUtils.html - isthere a reason why you don't want to use the glibmm version?

FILE_TEST_IS_REGULAR |   FILE_TEST_IS_SYMLINK |   FILE_TEST_IS_DIR |   FILE_TEST_IS_EXECUTABLE |   FILE_TEST_EXISTS

txe commented 5 years ago

Hi @giuspen, I just didn't run into Glib::file_test when I had been looking for something similar in gtk and I had to use std::filesystem. So there is no reason to use filesystem now.

giuspen commented 5 years ago

Great, thanks @txe

txe commented 5 years ago

Hi @giuspen , I've got a question: In the app, for some reason, instead of just changing an anchor widget property, a new widget with needed properties is created on the same place. Is it done for undo/redo functionality? For example, changing width/height of codebox in codebox_change_width_height is doing that, while I might try something like _scrolledwindow.set_size_request.

giuspen commented 5 years ago

Hi @txe, something was done for simplicity, something just because I keep learning and at that time that was not done right. You are more than welcome to rework an implementation that is inefficient. I also have changed the way the widgets nested into the text buffer are handled for example during the porting to gtkmm3, it's a good chance to make it right. I don't think it was for undo/redo specifically because there is a function to manually call to trigger a new state when no automatic event does it (like buffer modified or deleted)

txe commented 4 years ago

HI @giuspen, how are you? I've got a few days to contribute to the project, is there something I can do?

giuspen commented 4 years ago

Hi @txe it's great to hear from you! The beginning of 2020 has not been the best one for me but not too bad anyway, just after a good boost during the Christmas holidays I can hardly sit at the pc in these days, hope to get back to more relaxing days soon. I was thinking to take care about the undo/redo machine myself and proposing you the print/export to PDF if ok for you but literally anything still missing is open for business. I hope all is good with you and as I said before I would be happy to buy you at least few dinners out :)

txe commented 4 years ago

I'm fine, thanks! BTW, I've been to Italy, visited Avellino (hi to Tony Soprano:)), ancient Pompeii, Rome and Venice. Never seen so many mountains before, and high speed trains are really something. And, fortunately, I could get out from Venice just minutes before rains delayed all flights from there :) So, back to the proposal, I'll take a look at print/export PDF and about that payment, I'll send in an email when I figure out how to do it.