-
Notifications
You must be signed in to change notification settings - Fork 95
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
pythonqt_generator modified argument handling, make generate #124
Conversation
This adds extra arguments to control precisely what gets built and to enable use of the generator from within the makefile. Signed-off-by: John Bowler <[email protected]>
Adds context to any ReportHandler::reportError messages by calling ReportError::setContext at appropriate points in main.cpp. Also adds a missing fflush before the actual generate. Signed-off-by: John Bowler <[email protected]>
This adds 'make generate' which correctly handles rebuilding of the resources if they have been modified before generating the generated_cpp directory. This fixes the problem where editing typesystem_*.xml then running pythonqt_generator does nothing; the typesystem_*.xml files are compiled into the generator and 'make' must be used to rebuild the generator before the changes can be seen. Signed-off-by: John Bowler <[email protected]>
Thanks - I will check this tomorrow (too sleepy now...)! |
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.
Looks quite good to me, thanks! @usiems - could you also have a look?
Looks good to me so far. |
I.e. replace the hardcoded "path_splitter" by the appropriate Qt constexpr (introduced in Qt5.6). Signed-off-by: John Bowler <[email protected]>
Signed-off-by: John Bowler <[email protected]>
Signed-off-by: John Bowler <[email protected]>
sep = ","; | ||
} | ||
const QString errMsg("WARNING: missing core Qt modules: Qt" + | ||
requiredModules.join(",Qt")); |
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.
Nitpicking: If you want it as before (with the space) you need join(", Qt"))
.
This is getting too complicated and there is too much change of a merge conflict with other forks. |
Hm, I was actually going to merge this... this last nitpicking was not really important. All changes that are pure refactoring (like the |
Just drop it; I've convinced myself that the core-modules stuff is irrelevant because it, in fact, does nothing. I doubt any system really installs Qt headers with a repeated module directory, e.g. $QTDIR/QtCore/QtCore/qcoreapplication I can run pythonqt_generator by hand and get the same results. The real changes I need are in the error reporting; I only submitted this PR because I did it first so I'm going to have to spend some time de-merging the changes in my own branches/stashes. |
Ok - in case we still decide that the functionality is needed, we can recreate it from this PR... thanks! |
This adds 'make generate' to generator/generator.pro using modified argument handling so that the Qt installed header directory can be used to build a new generated_cpp. This is created in generator/ to avoid overwriting an existing user generated_cpp at the top level.
It also adds setContext calls to ReportHandler to improve the warning messages and calls fflush(stdout) at appropriate moments to fix the confusing mis-ordering of the pythonqt_generator output when it is piped to a file.