Skip to content
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

LS: Add completions of struct members #6567

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Conversation

integraledelebesgue
Copy link
Collaborator

@integraledelebesgue integraledelebesgue commented Oct 31, 2024

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

@reviewable-StarkWare
Copy link

This change is Reviewable

@integraledelebesgue integraledelebesgue changed the title Added struct members completions LS: Add completions of struct members Oct 31, 2024
Copy link
Collaborator

@piotmag769 piotmag769 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @mkaput, and @orizi)


crates/cairo-lang-language-server/tests/test_data/completions/structs.txt line 25 at r1 (raw file):

            x: 0x0,
            y: 0x0,
            z: 0x0

I guess missing here?


crates/cairo-lang-language-server/tests/test_data/completions/structs.txt line 40 at r1 (raw file):

            x: 0x0,
            <caret>
            ..s

To make sure it doesn't work in the second case

Suggestion:

            <caret>
            <caret>..s

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @orizi)


crates/cairo-lang-language-server/tests/test_data/completions/structs.txt line 47 at r1 (raw file):

mod happy_cases {
    use super::some_module::Struct;
    

please trim whitespace in empty lines


crates/cairo-lang-language-server/tests/test_data/completions/structs.txt line 62 at r1 (raw file):

            x: 0x0,
            <caret>
        }

consider putting all of these in single line so that source line contexts will make sense

Suggestion:

        let a = Struct { <caret> };

        let b = Struct { y: 0x0, <caret> };

        let c = Struct { y: 0x0, x: 0x0, <caret> }

Copy link
Collaborator Author

@integraledelebesgue integraledelebesgue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)


crates/cairo-lang-language-server/tests/test_data/completions/structs.txt line 25 at r1 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

I guess missing here?

This struct is instantiated only to be used below in a "tail": ..s

Copy link
Collaborator

@piotmag769 piotmag769 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @orizi)

Copy link
Collaborator Author

@integraledelebesgue integraledelebesgue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)


crates/cairo-lang-language-server/tests/test_data/completions/structs.txt line 62 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

consider putting all of these in single line so that source line contexts will make sense

Done.

Copy link
Collaborator Author

@integraledelebesgue integraledelebesgue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)


crates/cairo-lang-language-server/tests/test_data/completions/structs.txt line 47 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

please trim whitespace in empty lines

Done.

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae, @Draggu, and @orizi)

Copy link
Collaborator

@piotmag769 piotmag769 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae, @Draggu, and @orizi)

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae and @Draggu)

@integraledelebesgue integraledelebesgue force-pushed the spr/main/9fb1df2c branch 2 times, most recently from ed3b2f8 to b329f4c Compare November 5, 2024 17:08
Copy link
Collaborator

@Draggu Draggu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 4 files at r1, 1 of 1 files at r2, 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae)

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae)

@integraledelebesgue integraledelebesgue force-pushed the spr/main/9fb1df2c branch 2 times, most recently from 030cc39 to ef74001 Compare November 8, 2024 12:16
Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae)

Copy link
Collaborator

@piotmag769 piotmag769 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r5.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae and @integraledelebesgue)


crates/cairo-lang-language-server/src/ide/completion/completions.rs line 363 at r5 (raw file):

    let struct_members =
        concrete_struct_members(db, constructor_semantic_expr.concrete_struct_id).ok()?;

This is a query, use it as one

Suggestion:

db.concrete_struct_members(constructor_semantic_expr.concrete_struct_id)

Copy link
Collaborator

@piotmag769 piotmag769 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae and @integraledelebesgue)

Copy link
Collaborator Author

@integraledelebesgue integraledelebesgue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @Arcticae, @mkaput, and @piotmag769)


crates/cairo-lang-language-server/src/ide/completion/completions.rs line 363 at r5 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

This is a query, use it as one

Done

Copy link
Collaborator

@piotmag769 piotmag769 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae)

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae)

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Arcticae)

@integraledelebesgue integraledelebesgue changed the base branch from spr/main/46f1dadf to main November 14, 2024 17:41
@integraledelebesgue integraledelebesgue added this pull request to the merge queue Nov 14, 2024
Merged via the queue into main with commit 8fde3ce Nov 14, 2024
48 checks passed
@orizi orizi deleted the spr/main/9fb1df2c branch November 28, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: LSP completion dialog should only recommend members of a struct
6 participants