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

Inspect - don't exclude valid scope members declared after cursor position #61

Closed
mattmasson opened this issue Oct 2, 2019 · 3 comments
Assignees

Comments

@mattmasson
Copy link
Member

I'm not sure why we are filtering out members that are declared after the current position, since they would still be valid identifier suggestions (unless I am misunderstanding the intended use of the scope member).

Unit test:

it(`section foo; a = 1; b = |2; c = 1;`, () => {
    const [text, position] = textWithPosition(`section foo; a = 1; b = |2; c = 1;`);
    const expected: AbridgedInspection = {
        nodes: [],
        scope: [`a`, `c`],
    };
    expectParseOkAbridgedInspectionEqual(text, position, expected);
});
@mattmasson mattmasson self-assigned this Oct 2, 2019
@mattmasson
Copy link
Member Author

mattmasson commented Oct 2, 2019

I'm wondering if I'm misunderstanding the purpose of scope.
I tried removing the position check in inspectIdentifier, but that ended up breaking a number of valid test cases. So clearly a different or smarter approach is required.

case 1: (x|, y) => z

Removing the position check added y to the current scope, which isn't what we want.

If scope is for intellisense, then x shouldn't be included. If scope is used to represent a hierarchy of symbols, then we need an easy way to determine which scope members should be excluded from intellisense results (i.e. what assignmentKeyNodeIdMap is doing now).

case 2: s|ection foo; x = 1; y = 2;

Since the cursor is on the section keyword/declaration, we shouldn't have any members in scope.

@mattmasson
Copy link
Member Author

I resolved case 1 by not calling inspectIdentifier when processing the section.

For case 2, I updated the related test cases and added TODO comments.

I think this scenario is related to #56 in that there are certain cursor positions where we should be returning an empty scope, or providing additional context so the intellisense layer knows that the current list of identifiers shouldn't be included in suggestions.

@JordanBoltonMN
Copy link
Contributor

Should be resolved in #70

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

No branches or pull requests

2 participants