Patch implementing Bullet and numeric lists and hyper links in qt5 an…#2
Patch implementing Bullet and numeric lists and hyper links in qt5 an…#2jesusrmx wants to merge 1 commit intoskalogryz:masterfrom
Conversation
skalogryz
left a comment
skalogryz
left a comment
There was a problem hiding this comment.
The change is definetly very desired overall, but it seems like it was done parts
The initial approach was to implement Gtk2 (by extending RichMemo APIs, due to limitations of Gtk2 itself)
But the later addition of Qt5 doesn't require all was extensions (as Qt5 rich editor is smarter than Gtk2).
Can you create a separate change request for RTF reading part only? :)
| property OnPrintAction: TPrintActionEvent read fOnPrintAction write fOnPrintAction; | ||
| property OnLinkAction: TLinkActionEvent read fOnLinkAction write fOnLinkAction; | ||
| property CanRedo: Boolean read GetCanRedo; | ||
| property FastLoad: boolean read fFastLoad write fFastLoad; |
There was a problem hiding this comment.
"FastLoad" property is my biggest concern. Doesn't look like a nice API design.
I can understand if it's added as a parameter to LoadRTF, but adding it as a property seems awkward to me,
Because It doesn't affect the behavior of the control in general, it only comes into effect when loading data.
There was a problem hiding this comment.
Indeed, FastLoad was not supposed to be exposed to the user, I will remove it and replace it with a developer flag, I'm not ready to remove the previous default code because I intended to do some tests and want it for reference. The chunked (fast) loading is what will remain.
| end; | ||
|
|
||
| class function TWSCustomRichMemo.HandleKeyDown(const AWinControl: TWinControl; | ||
| key: word): boolean; |
There was a problem hiding this comment.
Is this TWSCustomRichMemo.HandleKeyDown() actually Gtk2 handle key?
I can see it's has a different implementation for Qt5.
Should the default implementation return false (just like Win version does) and this implementation move to Gtk2?
There was a problem hiding this comment.
It is, I first implemented the gtk part without knowing that qt already do some things (and I have not touched the cocoa part yet), the intention was to handle bullet and numbered lists managing in a widgetset independent way, but it turned out to be not necessary, I hope cocoa also will not need it and then it will be moved to the gtk side.
There was a problem hiding this comment.
how about making a qt5 implentation first?
cocoa does do everything by itself, just like Windows and Qt.
There was a problem hiding this comment.
nice!, then that solves it. I will move everything to gtk
You mean to split the parser/loading/saving from the rest?, I thought about it before submitting the pull request to split it in several parts, but in the end I didn't see gain on it. I cannot submit first the reading part, because it needs the Link and Lists support, but submitting first the other part will make the previous reading part not useful as there is not knowledge or support from the new part, also one can say that the gtk part could be splitted from the qt part, but as I say there seems to be no real gain on it. Yet if you prefer it that way I can try to do it. |
|
Ahem, I how do I do this?, I intend to start again by committing step by step, but how do I start? should I remove my fork or reset hard to the starting point? should first the pull request be cancelled? would that remove this conversation? .... |
|
you might want to create a branch on your repository (the branch should be on the code w/o changes done). |
Thus I'd think that Qt5 update and RTF reader update could be delivered with a single pull request. Gtk2 requires rework (to put everything insider of gtk2wsrichmemo) and should be pull requested separately |
I don't know if QT5 can read and write .rtf format files, I could not find it (but I have to admit that I didn't search very hard), if you have other info please share it. The reference here was the windows writer as it its closed source we have to play with its restrictions, then the internal QT5 reader/writer (if it exists) will require to load and save in this same format. My patches make sure that read/write works cross widgetsets (or more precisely: except for windows and cocoa it seems)
The current implementation is completely agnostic on the widgetsets (I think it will work even in windows if the windows version of GetStyleRange is modified to process paragraphs in order to dectect numbered lists) so there is no way isolate only the Qt5 reader/writer part (or the gtk2 part) |
I will try, it can be made in four stages. a)Qt5, b)Gtk2 c)Win d)Read/Write, thus can be abcd, adbc, bdac, any combination as you prefer, well except for dxxx because that would not compile, also d cannot be divided in Qt5 or Gtk parts btw. |
|
2 participants
…d gtk2.