Skip to content

Conversation

@StanFromIreland
Copy link
Member

@StanFromIreland StanFromIreland commented May 5, 2025

| '*' param a='=' { RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "var-positional argument cannot have default value") }
| '*' (param_no_default | ',') param_maybe_default* a='*' (param_no_default | ',') {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "* argument may appear only once") }
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "* marker may appear only once") }
Copy link
Member Author

Choose a reason for hiding this comment

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

I liked @picnixz suggestion though there may be an even better word?

Copy link

@Kivooeo Kivooeo May 5, 2025

Choose a reason for hiding this comment

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

imo, "an asterisk may appear only once" is as clear as possible and will not cause any confusion in terms of terminology

Copy link
Member

Choose a reason for hiding this comment

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

Other error messages refer to / as just / and to * as just *. On other hand, * marker can be confused with bare *. So I think that it is better to use just * here.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

There are more:

    def f(/, *a, b): pass
          ^
SyntaxError: at least one argument must precede /
    def f(**a, b): pass
               ^
SyntaxError: arguments cannot follow var-keyword argument

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

More examples:

    def f(/, a, /, b): pass
          ^
SyntaxError: at least one argument must precede /
    def f(a, a): pass
             ^
SyntaxError: duplicate argument 'a' in function definition

| '*' param a='=' { RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "var-positional argument cannot have default value") }
| '*' (param_no_default | ',') param_maybe_default* a='*' (param_no_default | ',') {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "* argument may appear only once") }
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "* marker may appear only once") }
Copy link
Member

Choose a reason for hiding this comment

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

Other error messages refer to / as just / and to * as just *. On other hand, * marker can be confused with bare *. So I think that it is better to use just * here.

@picnixz
Copy link
Member

picnixz commented May 9, 2025

The rest is in #133382, but maybe we can do all in one PR? I suggested two because of semantics, but maybe it's better to do everything at once (in this case, let's use #133382 instead and close the other issue as well)

@serhiy-storchaka
Copy link
Member

I suggest to do all in one PR. I do not see semantic difference. There are other error messages that need corrections, see my examples.

@picnixz
Copy link
Member

picnixz commented May 9, 2025

I'll close this one and the corresponding issue. We'll work with one issue and one PR instead.

@picnixz picnixz closed this May 9, 2025
@StanFromIreland StanFromIreland deleted the marker-err branch May 9, 2025 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants