qt: add preserve_paths keyword to functions #11795
Conversation
|
Do you think there is value in adding a test? Overall this looks good to me though. @punytroll do you wanna test this out to make sure your case is covered too? |
Funny misspelling :) |
Oups! Sorry, I'm not English native. What is the correct word? |
|
Seems weird to mention in the commit message which other PR this supersedes. But also I don't see how it is supposed to supersede the other PR? |
|
@bruchar1 it is supersedes |
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
e66b22c
to
5b84fe6
Compare
| @@ -55,11 +55,15 @@ foreach qt : ['qt4', 'qt5', 'qt6'] | |||
| method : get_option('method') | |||
| ) | |||
| # XML files that need to be compiled with the uic tol. | |||
| prep += qtmodule.compile_ui(sources : 'mainWindow.ui', method: get_option('method')) | |||
| prep += qtmodule.compile_ui( | |||
There was a problem hiding this comment.
Is it possible to extend the test to also use compile_moc?
There was a problem hiding this comment.
TBH, I'm not familiar enough with Qt to know now to do a simple test for that. My usecase was for .ui files. I just extended it to moc as well, but in our codebase, it seems the current moc_compile is working without modifications.
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
5b84fe6
to
0976fbc
Compare
|
Seems like this comment is all that is left to answer #11795 (comment) |
|
@eli-schwartz did you have anything more to say on this PR? |
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
0976fbc
to
066fedb
Compare
|
CI failure seems unrelated given it is a fortran test that failed |
|
@xclaesse @eli-schwartz does this PR look good to either of you? Could be a good vacation present to get this one in! :) |
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
066fedb
to
ec3f458
Compare
|
Added to the 1.3.0 milestone, so it isn't left in the dust again. |
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
ec3f458
to
b66adf5
Compare
bruchar1
changed the title
|
I just simplified it to use a bool instead of a path. Preserving paths from current source dir is probably the only real usecase. If somebody really needs to specify a different path, we could extend it later (or he can simply use a custom_target) |
|
I think that was a good change. It matches with |
Thanks for this change. The other solution was, of course, way more general, but, I think, pointlessly so. Thanks, for giving it to the other Qt commands as well! |
|
I've tested this PR for my use case (qt.compile_moc() with headers) as well and it works beautifully. I hope, this makes it into 1.3. |
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
b66adf5
to
9573662
Compare
|
@eli-schwartz @xclaesse Is there anything preventing merging this? |
| @@ -49,6 +54,11 @@ It takes no positional arguments, and the following keyword arguments: | |||
| - `dependencies`: dependency objects whose include directories are used by moc. | |||
| - `include_directories` (string | IncludeDirectory)[]: A list of `include_directory()` | |||
| objects used when transpiling the .moc files | |||
| - `preserve_paths` bool: *New in 1.4.0*. If `true`, specifies that the output | |||
There was a problem hiding this comment.
I feel like we typically use Since and not New in. Are you matching something else?
There was a problem hiding this comment.
This file is full of New in ..., but Since is good for me!
There was a problem hiding this comment.
Oh I was trying to say if you're being consistent, that is good with me. But whatever you want.
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
9573662
to
fb6783b
Compare
This allow to generate ui and moc under subdirectories, as this is allowed with generic generators.
bruchar1
force-pushed
the
qt-preserve-paths
branch
from
fb6783b
to
444850f
Compare
supersedes #10995, as it also handle
ui_compileandpreprocess, and expose the keyword argument, instead of setting a different default value.This allow to generate ui and moc under subdirectories, as this is allowed with generic generators.