draft: add "commented cursor" classes and use them

[jnikula]

This is some early drafting for passing cursors around. Basically add an abstraction for a "commented cursor" (naming sucks) that consists of two cursors, the comment and the actual object. Some of the intermediate steps are ugly, but I don't want to try to do a mass conversion either. Towards the end there's more clarity. parser.py is only about parsing and creating commentedcursors, commentedcursor.py is only about presenting a cleaner interface to the cursors clang gives us, and docstring.py is more about converting those commentedcursors to something presentable to Sphinx.

I guess there's not much point in detailed review at this point, but maybe have a look at the end result, and see if that is the direction we should be going? I think it does look neater, and looks like will be easier to understand.

[BrunoMSantos]

Juicy. I'll need to set aside some time for this, but out of curiosity, did you find anything useful in the work I had done earlier? I'm guessing not.

[jnikula]

Juicy. I'll need to set aside some time for this, but out of curiosity, did you find anything useful in the work I had done earlier? I'm guessing not.

I forgot to check 🤦

I just started on something else, and one thing lead to another...

[BrunoMSantos]

Juicy. I'll need to set aside some time for this, but out of curiosity, did you find anything useful in the work I had done earlier? I'm guessing not.

I forgot to check 🤦

I just started on something else, and one thing lead to another...

Ahah, no worries, I also didn't advertise it loud enough. Likelier than not, there isn't much in there of any real interest anyway.

[BrunoMSantos]

I guess there's not much point in detailed review at this point, but maybe have a look at the end result, and see if that is the direction we should be going? I think it does look neater, and looks like will be easier to understand.

Yes! 100% yes. I read it through a couple of times and the overall approach matches what we discussed in #180 and I like it!

A major difference in the version I had played around in is that I had a DocCursor that did everything instead of multiple cursor classes, one for each 'cursor type'. That may not be a bad idea, though I'm not really sold, especially as 'cursor types' don't really match what a user would expect (CompoundDecl... what is that? Still need get_type() to know). I think it may also be better to have a cursor type that always presents the same API (like Clang's own cursor) and if (e.g.) you do a .get_args() on a structure cursor, you don't get an exception for a non existing member but either None or maybe even a custom exception stating 'structs have no arguments'. Though this last bit can be done as well by placing all those things in the base class.

On the other hand I was getting to a point in my version where the parser became / would become a tiny loop building DocCursor(cursor), ultimately not even checking cursor.kind or even going into a structure: the DocCursor would provide methods to iterate over the members of the structure in that case and returning DocCursor of its own, just like the Clang one... Maybe not a bad thing? I can think of some advantages actually. But I'll wait for your 1st impressions. The point is I'm not 100% set in either direction yet.

There are other small differences, but nothing to consider right now.

And as for the name... DocCursor is shorter, but I can't say I prefer it exactly. It's just different. I also considered HawkCursor and such, but don't really like it.

Other than that, I left my branch in a really bad state, I had a look over and I don't think there's much to take from it, especially with how old an out of date it is. Yours on the other hand seems like a good sketch of how it could go in. How would you want to proceed here? Do you want to polish this up a bit? Should I?

[BrunoMSantos]

@jnikula Not sure you saw this or what you think of it. I've played with it a bit and I'm building up a branch according to my comments above. It's still a work in progress, but I wanted to mention it in case we're both doing the same thing.

You can follow it here.

[jnikula]

@jnikula Not sure you saw this or what you think of it. I've played with it a bit and I'm building up a branch according to my comments above. It's still a work in progress, but I wanted to mention it in case we're both doing the same thing.

Sorry, I saw your comment, it deserved a decent reply I didn't have time for. 😞

One thing was that I thought multiple cursor classes really helped me wrap my head around the whole thing when reading the code.

[BrunoMSantos]

@jnikula Not sure you saw this or what you think of it. I've played with it a bit and I'm building up a branch according to my comments above. It's still a work in progress, but I wanted to mention it in case we're both doing the same thing.

Sorry, I saw your comment, it deserved a decent reply I didn't have time for. 😞

No worries of course.

One thing was that I thought multiple cursor classes really helped me wrap my head around the whole thing when reading the code.

Yeah, I get that, but still think I'm on to something regarding the cursor type (as in which class it is) and cursor kind, which don't match and would just be odd. And this is to be public facing API if we pass the cursors to the extensions.

Also, I quite like the elegant way we can generate the cursors here and use get_children() on them directly.

Let it stew a bit longer then and let me know how you see that playing out. I'll keep playing with my idea for now.

[BrunoMSantos]

Forgot to mention, I'm thinking of maybe using multiple classes actually, but internal ones, used compositing style instead of inheritance. That would allow us to manage readability a bit better while maintaining a single point of entry API wise I think.

This is not reflected in my branch at the moment though.

[jnikula]

@BrunoMSantos I looked at your branch, and it looks nice! It's just that there's not enough to tell where it'll go next... I'd like to see the cleanup to parser.py, something similar to what I do in ac799b1 (being too impatient am I?!)

I wonder if we should use different variable names for Cursor and DocCursor. Just to be clear which is which.

Anyway, I'm not hung up on the implementation I've got here. I just wanted to test where we could go with this, and I think something like this is the way to go. If you've got the time and energy to take this forward, by all means do!

[jnikula]

@BrunoMSantos Please shove https://gitlab.com/bms-contrib/hawkmoth/-/commit/458f797b4c27d22594a0d9c505035d01db8cb836 to a branch and send a pull request!

[BrunoMSantos]

@BrunoMSantos I looked at your branch, and it looks nice! It's just that there's not enough to tell where it'll go next... I'd like to see the cleanup to parser.py, something similar to what I do in ac799b1 (being too impatient am I?!)

Literally working on that 😅
I'll try to push often, but life's short as always. Pacing myself!

I wonder if we should use different variable names for Cursor and DocCursor. Just to be clear which is which.

I did, didn't I? At least where they are mixed, in parse: cc stands for clang cursor and is short lived. From that point onwards they're all cursor of DocCursor type. There may be a few cursors in the comment extraction, but that's a different scope. I'll have a look anyway.

Anyway, I'm not hung up on the implementation I've got here. I just wanted to test where we could go with this, and I think something like this is the way to go. If you've got the time and energy to take this forward, by all means do!

👍

[BrunoMSantos]

@BrunoMSantos Please shove https://gitlab.com/bms-contrib/hawkmoth/-/commit/458f797b4c27d22594a0d9c505035d01db8cb836 to a branch and send a pull request!

Yup, that was the intention, I was just not in a hurry, but I'll do that now ;)

[jnikula]

I wonder if we should use different variable names for Cursor and DocCursor. Just to be clear which is which.

I did, didn't I? At least where they are mixed, in parse: cc stands for clang cursor and is short lived. From that point onwards they're all cursor of DocCursor type. There may be a few cursors in the comment extraction, but that's a different scope. I'll have a look anyway.

I just didn't realize cursor became the DocCursor thing. My bad.

[jnikula]

I'm closing this one in favour of #217