-
Notifications
You must be signed in to change notification settings - Fork 34
(very WIP) KF6 port #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Unfortunately, I haven't had time to continue working on this after the general cleanups from a while back. My next item on the TODO list was porting to I'm also not sure if it's desired to keep the codebase working on both Qt5 and Qt6 (before it was working on both Qt4 and Qt5). It definitely requires some discussion and planning 😁 |
|
Hi, could we have some reply from the maintainer of the application? |
|
Original author here. I'm not sure if @jfmcarreira is still active, he was maintaining ktikz mostly. Maybe wait a few days to give him a chance to reply. If he doesn't, I'd say if you are up for moving forward @alexfikl feel free to. Supporting two versions of Qt is nice, but given the maturity of the application, I'd argue its fine to only support the latest version for new releases, if that makes development easier. Security fixes can then still be backported. If you need repository permissions, just shoot me an email. Thank you for your contributions! |
|
I would agree that it might be easy moving foward to keep only Qt6 support. I believe the application is mature enough and not seen new features appearing that it is deal breaker to keep the current version if you dont have Qt6 available. Sorry for not following along. Nowadays, I do not use ktikz as often as I used to, and not using Linux on my daily driver is hard to keep mantaining it. |
|
@flying-sheep Hello could you please remove the vscode project files from the PR. I will try to test and give you some feedback on the PR |
alexfikl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good already! 😁
Things that seem still left to do:
- I can confirm that it gives a segmentation fault on exit (and maybe some other situations), but haven't had the time to look at it.
- The CI will need some massaging to get the qt6 dependencies installed correctly. Need help with that?
| for (const auto &rule : m_highlightingRules) { | ||
| // const QRegExp expression(rule.pattern); | ||
| // int index = text.indexOf(expression); | ||
| QRegExp expression(rule.pattern); | ||
| int index = expression.indexIn(text); | ||
| while (index >= 0) { | ||
| const int length = expression.matchedLength(); | ||
| auto m = rule.pattern.match(text); | ||
| while (m.hasMatch()) { | ||
| int index = m.capturedStart(); | ||
| const int length = m.capturedLength(); | ||
| setFormat(index, length, rule.format); | ||
| // index = text.indexOf(expression, index + length); | ||
| index = expression.indexIn(text, index + length); | ||
| m = rule.pattern.match(text, m.capturedEnd()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could maybe work better with global matching?
https://doc.qt.io/qt-6/qregularexpression.html#global-matching
| this->configureStreamDecoding(in); | ||
| m_tikzQtEditorView->editor()->setPlainText(in.readAll()); | ||
| setCurrentEncoding(in.codec()); | ||
| //setCurrentEncoding(in.codec()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This codec stuff probably needs fixing before merging. I'm not sure what the current way of working is..
| cci->registerCompletionModel(m_completion); | ||
| } | ||
| m_completion = new TikzKTextEditorCompletion(this); | ||
| m_documentView->registerCompletionModel(m_completion); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is at fault, but I tried writing some code and completion doesn't seem to be working.
| install(TARGETS ktikzpart DESTINATION ${KDE_INSTALL_PLUGINDIR}) | ||
| install(FILES ktikzpart.rc DESTINATION ${KTIKZPART_DATA_INSTALL_DIR}) | ||
| install(FILES ktikzpart.desktop DESTINATION ${KDE_INSTALL_KSERVICES5DIR}) | ||
| #install(FILES ktikzpart.desktop DESTINATION ${KDE_INSTALL_KSERVICES5DIR}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can probably just be removed. The json metadata gets compiled into the library, as far as I know.
Thank you for the kind words! I still use ktikz quite often, so I would very much like to see it stick around (thus my initial burst last year to get it closer to Qt6). I'm not a C++/Qt developer though, so I'm not very confident I can help much with maintaining / fixing bugs :( |
|
More than a few days have passed ... ? |
|
Hello, I can try integrate this and see if I could find the segfault problem (is it still a problem?) However, as I am not a daily user of this app and not using linux as a daily driver, it is being hard to keep up with recent changes in KDE. I might need help maintaining this repo. maybe @fhackenberger could give permission for others to push code. |
|
The crash was on the KTextEditor. I will disable it in master |
|
Hopefully temporarily! The KTextEditor-base build is the only one I care about. |
At this point I’m basically just changing things until it compiles.
It does currently, but crashes with a “pure virtual method called” segfault on close, and I’m too bad with C++ to have an inkling about why that might be.
I’d be happy to help getting this into shape, but I’m also happy if you just take it as inspiration.