Skip to content

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Nov 5, 2018

@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Nov 5, 2018
@serhiy-storchaka serhiy-storchaka force-pushed the ast-assignment-error-messages branch from 495bf38 to 29e36ef Compare November 5, 2018 20:07
Python/ast.c Outdated
};

static int
is_forbidden_name(const char *name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not duplicate this by having ast_for_call turn the name into an identifier and passing it to forbidden_name?

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 tried this initially. But this needed to add more complex code at calling places if we want to raise the same error "expression cannot contain assignment" for both f(0 = a) and f(False = a).

But if "cannot assign to False" is good enough or even better for the latter case, the code can be simplified.

ast_error(c, chch,
"keyword can't be an expression");
"expression cannot contain assignment, "
"perhaps you meant \"==\"?");
Copy link
Contributor

Choose a reason for hiding this comment

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

As a matter of style, I don't think we should use the second person ("you") in error messages.

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 agree. Could you please suggest better error message?

I based this error message on the error messages for Python 2 style print. There are also other error messages that contain "you":

  • "Missing parentheses in call to 'print'. Did you mean print(%U%s)?"
  • "unsupported operand type(s) for %.100s: '%.100s' and '%.100s'. Did you mean \"print(<message>, file=<output_stream>)\"?"
  • "if you give only one argument to maketrans it must be a dict"
  • "new-line character seen in unquoted field - do you need to open the file in universal-newline mode?"
  • "iterator should return strings, not %.200s (did you open the file in text mode?)"
  • "utime: you may specify either 'times' or 'ns' but not both"
  • "Binding %d has no name, but you supplied a dictionary (which has only names)."

Perhaps we should make them less personalized too. But this is other issue.

@serhiy-storchaka serhiy-storchaka merged commit 97f1efb into python:master Nov 20, 2018
@serhiy-storchaka serhiy-storchaka deleted the ast-assignment-error-messages branch November 20, 2018 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants