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

Add positional-only parameters to parser, analysis, LS, etc #1614

Merged
merged 27 commits into from Oct 3, 2019

Conversation

@jakebailey
Copy link
Member

commented Oct 1, 2019

Fixes #1511.

Still needs some testing in the analysis past ArgumentSet specific ones, but it's nearly there.

jakebailey added 24 commits Sep 27, 2019
@jakebailey jakebailey changed the title [WIP] Add positional-only parameters to parser, analysis, LS, etc Add positional-only parameters to parser, analysis, LS, etc Oct 3, 2019
@jakebailey

This comment has been minimized.

Copy link
Member Author

commented Oct 3, 2019

Ready for a look, I think.

@@ -2120,6 +2145,19 @@ struct Name {
ReportSyntaxError(p.StartIndex, p.EndIndex, Resources.DuplicateArgsDoubleArgumentErrorMsg);//duplicate ** args arguments
}
seenDictArg = true;
} else if (p.Kind == ParameterKind.PositionalOnlyMarker) {
if (seenPositional) {

This comment has been minimized.

Copy link
@MikhailArkhipov

MikhailArkhipov Oct 3, 2019

Member

Do we want to show all errors, i.e. if it has several issues like annotation, default and something else?

This comment has been minimized.

Copy link
@jakebailey

jakebailey Oct 3, 2019

Author Member

I think the convention in the parser is to try and only show one error at a time per-character (if that makes sense), such that any given place will only be squiggled the one time.

This comment has been minimized.

Copy link
@MikhailArkhipov

MikhailArkhipov Oct 3, 2019

Member

VS squiggles multiple so user can fix all errors rather than fix one, then see another. Technically squiggles should be different, i.e. annotation under :, default under =. Up to you.

This comment has been minimized.

Copy link
@jakebailey

jakebailey Oct 3, 2019

Author Member

I'd prefer to stick with this for now, and come back to improve the errors overall if they overlap (since we have a number of cases like this already).

@jakebailey jakebailey merged commit c4f590b into microsoft:master Oct 3, 2019
1 check passed
1 check passed
license/cla All CLA requirements met.
Details
@jakebailey jakebailey deleted the jakebailey:positional-parameters branch Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.