Regression in v1.1.3 for hover on struct field declarations

[automaticp]

Okay, now it seems that solving #23 broke hover for struct fields:

Rolled back to v1.1.2 and works fine there. The hover and autocomplete there just don't display anything.

(Can't reopen the old issue, since it wasn't me who closed it, so here's a new one.)

[nolanderc]

What seems to be happening is that we start looking for field at the actual identifier, walking the parse tree upwards, first encountering the field declaration int field, then the struct-block { int field; } and finally the global declaration struct A { int field; };. I don't see any easy fixes here, except just removing any duplicates before sending completions to the editor.

Again, this would be fixed with top-down name-resolution (really, should have just implemented that from the start :sweat_smile:).

[automaticp]

I was about to reply to your message with something, but then I found another bug :disappointed:.

How about them shapeshifting types:

When hovered in the middle of the identifier, and...

when hovered at the end.

[nolanderc]

Haha, that's awesome. Clearly I got my work cut out for me :smile:

[automaticp]

As for what I wanted to say:

I feel like it's probably best to put some work into building a comprehensive set of tests for hover + autocomplete, so that the ripple effect from fixes would be visible.

Or else I'm afraid I'll keep throwing these minor bugs at you and that would result in a messier codebase as you wouldn't get enough time to think of the proper solution or see the bigger picture and as more obscure bugs would creep in because of it like it's some kind of whack-a-mole game.

Plus should help a ton when the big rewrite comes.

[nolanderc]

Yes, totally agree. It makes more sense to just let these issues be (for now), and focus on the rewrite instead of adding small patches here and there.

Guess I'm just suffering from the sunk cost fallacy, so thanks for talking me out of it :slightly_smiling_face:

[automaticp]

Great, I opened an issue on the tests (#26) for starters, feel free to chime in. I'll try to drop something in the following days, and I'll probably chill on the bug reports unless it's a crash or critical in another way. I'm already feeling a bit guilty from showering you with all of these minor bugs :cry:.

[nolanderc]

I'm already feeling a bit guilty from showering you with all of these minor bugs 😢

No worries, I enjoy figuring them out 😸

[automaticp]

Seems to be somewhat fixed in 6296f9388b68f975b1a41e637fa27e5d5a1030c5. Should the issue be closed or you want to wait until this is fixed too?

[nolanderc]

I have fixes for both of these in feat/top-down-semantics. See https://github.com/nolanderc/glsl_analyzer/commit/f4b65f1e0db13d9599b28545e63545a0cceb9dee for the second one. I might merge those in tonight.

[automaticp]

Oh, nice work in that branch! Got it.

[nolanderc]

Fixed in v1.2.0

[automaticp]

I am pretty sure this update did a lot of great things so I am so sorry for this screenshot

This only happens in the scope of SomeType.

[nolanderc]

Oh right, I actually implemented shadowing, but never enabled it :sweat_smile:

Will be fixed in v1.2.2 :slightly_smiling_face:

(you can close this if it works now)

[automaticp]

Great, that works now! Thanks.

I have a few other minor ones, I'll open an issue just to keep track of them and not forget about them before the tests for hover are in place.