feat: Add symbol listing for Qml - #79
Conversation
2454abe to
11c72d1
Compare
narnaud
left a comment
There was a problem hiding this comment.
This is on the right track. I didn't test it (I wonder if you see the symbols in the GUI yet), but the architecture definitely makes sense.
LeonMatthesKDAB
left a comment
There was a problem hiding this comment.
Overall looks like a reasonable way forward 👍
One additional thing to consider though is that the FunctionSymbol class also runs a few small queries to figure out the names and types of parameters (see the argumentsFromQueryMatch function).
However, that is of course also C++-specific and won't work with QML.
We should try to find a solution for injecting the right behavior there as well.
Also added some minor nitpicks.
11c72d1 to
1cad851
Compare
yes we can |
1cad851 to
548670e
Compare
LeonMatthesKDAB
left a comment
There was a problem hiding this comment.
The separation of concerns looks clean 👍
What I'd like to see addressed before merging:
- Find a way to make
FunctionSymbol::argumentscompatible with Qml.- Currently it's not capturing the individual parameters as
@parameter, so the FunctionSymbol will always report 0 arguments - The types of the arguments would be extracted incorrectly in the
FunctionSymbol::argumentsFromQueryMatchfunction, which would have to also differentiate between languages.
- Currently it's not capturing the individual parameters as
- Add tests for all types of QML symbol extraction. Currently this is entirely untested.
I guess we could also do these in follow-up PRs, but I don't expect much conflict in this part of the codebase at the moment, so it would be preferable to have everything in before merging.
|
Hm, I also took another look at the ClassSymbol. I guess that also won't work correctly, as it will probably associate all members from all sub-objects into itself. |
548670e to
e38b6b2
Compare
LeonMatthesKDAB
left a comment
There was a problem hiding this comment.
Going in the right direction, but the testing should be more thorough.
6beb17d to
d49b3ac
Compare
6495217 to
b2a332d
Compare
for more information, see https://pre-commit.ci
LeonMatthesKDAB
left a comment
There was a problem hiding this comment.
Looks good. We'll want to follow up on this with fixes for FunctionSymbol and ClassSymbol, but a very good start 🥳
symbol listing for Qml added.