Skip to content

Syntax errors branch#17

Open
krantou wants to merge 3 commits into
sourcelair:masterfrom
krantou:syntax-errors-branch
Open

Syntax errors branch#17
krantou wants to merge 3 commits into
sourcelair:masterfrom
krantou:syntax-errors-branch

Conversation

@krantou
Copy link
Copy Markdown
Contributor

@krantou krantou commented Aug 29, 2018

No description provided.

Comment thread env_spec.py Outdated
comment_regex = r"^(.+)\#(.+)$"

lines = env_spec_text.split("\n")
enumerated_list = list(enumerate(lines))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we do not have to wrap enumerate(...) in a list, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if I have only enumerated_list = (enumerate(lines)), I will get the enumerated object.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Won't the enumerated object work like a list in a for ... in loop?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will.My mistake.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a problem at all 😄!

Comment thread env_spec.py Outdated
if not is_variable_name_valid:
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid variable name.")
raise EnvSpecSyntaxError(
"SYNTAX ERROR: Invalid variable name.", line_number
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you change the error message to this please?

Invalid variable name; it should contain only latin alphanumeric characters, underscores and not start with a digit.

Comment thread env_spec.py Outdated
if not choices:
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid choices list.")
raise EnvSpecSyntaxError(
"SYNTAX ERROR: Invalid choices list.", line_number
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you change the error message to this please?

Invalid restricted choices syntax.

Comment thread env_spec.py Outdated
if default_value not in choices or default_value == "":
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid default value.")
raise EnvSpecSyntaxError(
"SYNTAX ERROR: Invalid default value.", line_number
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you change the error message to this please?

Invalid default value; it is not included in the provided restricted choices.

Comment thread env_spec.py Outdated

if env_spec_type not in valid_types_list:
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid type.")
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid type.", line_number)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you change the error message to this please?

Invalid variable type; it should be one of "bla", "blah", "blah".

P.S.: @krantou do not hardcode the valid types in the error message above, but construct the substring by joining the elements of valid_types_list.

Comment thread env_spec.py

if not is_variable_name_valid:
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid variable name.")
raise EnvSpecSyntaxError(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you change the error message to this please?

Invalid variable name; it should contain only latin alphanumeric characters, underscores and not start with a digit.

Also, why are we validating the variable name in two places?

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.

2 participants