-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
migrated to qt6 #429
base: master
Are you sure you want to change the base?
migrated to qt6 #429
Conversation
CMakeLists.txt
Outdated
Qt6 | ||
COMPONENTS ${QT_MODULES} LinguistTools | ||
REQUIRED) | ||
if(FLATPAK) | ||
find_package( | ||
Qt5 5.15 | ||
Qt6 |
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.
we have to find a way to support both
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.
Do we? What platforms/targets don't have Qt6 on the menu already and/or won't soon? It seems to be a project like this would benefit from staying on top of the toolkit/library game and any effort back-porting to older versions should be on an as-demanded basis and provided by the people that really want to see it happen for whatever reason. The main development effort should be on the latest stable releases when possible and this seems like a great opportunity to hit that target.
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.
Supporting both would mean lots of #ifdefs, a nightmare...
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 rebased the branch, maybe we can go just with Qt6
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.
@exactly-one-kas do you have anything against it?
e6d816f
to
ad2a208
Compare
@@ -13,8 +13,6 @@ | |||
#include <QMessageBox> | |||
|
|||
int main(int argc, char *argv[]) { | |||
Application::setAttribute(Qt::AA_EnableHighDpiScaling); | |||
Application::setAttribute(Qt::AA_UseHighDpiPixmaps); | |||
Application app(argc, argv, true); |
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.
Why is this anymore required?
@@ -505,7 +506,7 @@ class LfsPanel : public QWidget { | |||
watcher->deleteLater(); | |||
}); | |||
|
|||
watcher->setFuture(QtConcurrent::run(repo, &git::Repository::lfsTracked)); |
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 understand why you changed it, but I don't understand why it worked also the other way?
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.
Ok it does not compile the other way around, but the function definition looks same in Qt5 and Qt6
src/dialogs/DeleteBranchDialog.cpp
Outdated
@@ -61,8 +61,9 @@ DeleteBranchDialog::DeleteBranchDialog(const git::Branch &branch, | |||
|
|||
entry->setBusy(true); | |||
QStringList refspecs(QString(":%1").arg(upstreamName)); | |||
git::Result (git::Remote::*push)(git::Remote::Callbacks*, const QStringList&) = &git::Remote::push; |
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.
what is the benefit of this function pointer?
@exactly-one-kas do you think we still need win32? |
@exactly-one-kas Can you test and build this branch on Arch Linux? |
Mac: change CustomTheme_mac.mm use float Win: D:/a/Gittyup/Gittyup/src/index/indexer.cpp:61:3: error: no matching function for call to 'CreateDirectoryW' |
13cb439
to
706f288
Compare
@probonopd maybe you have an idea why the appimage builder does not find libqxcb.
I printed the paths: Show QTDIRbin Show QTDIR/pluginsdesigner Show QTDIR/plugins/platformslibqeglfs.so |
@Murmele not really, sorry. Haven't had time to investigate Qt6 yet. |
Ah thanks, then I will check it out and notify you about |
@probonopd It seems that it was due to the wildcard in the qt version. Just for your information: 00675a7 |
We can check the commit c0754a0e061352c8d8d4434952115796063c9401 from GitAhead where the port happened |
|
||
- version: 5.12.0 | ||
check_only: true | ||
- version: 6.5.3 |
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.
updating? Maybe it fixes the theme issues?
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 can try it when i have some time this or next week, but i don't think it will fix our problems here, it must have something to do with the custom theme Gittyup has because default ways of theming work in every qt6 version i tried.
Then we need to have a deeper look into it because GitAhead does not have this problem with Qt6 |
- cleanup - Fix usage of deprecated functions
I just tried to find the differences in GitAheads QT6 commit in order to fix our theming problems. First things first: Theme problems on MacOS:
Theme problems on Linux:
.git name problem:@Murmele This has nothing to do with this QT6 migration, we already talked about this somewhere else, am i correct? |
You mean #760? |
This reverts commit 918df65.
Yes, but is there already an issue for the "Use Ours", "Use Theirs" button color problem? This happens in Gittyup 1.4.0 on MacOS for me. |
@exactly-one-kas If you have some time can you please check out the Windows theming problem this branch has, i am unable to build openssl on windows.. |
I have just migrated a fork of Gittyup to Qt6.
It compiles (without deprecation warnings) and seems to run, but is still mostly untested (on Manjaro-KDE-Linux only).