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.

Sign up for GitHub

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

Syntax errors branch #17

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+59 −21
Open

Syntax errors branch #17

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bd1208
need changes about linenumber
krantou Aug 28, 2018
fc0431b
changed parse function
krantou Aug 29, 2018
a385aa1
changed error messages
krantou Aug 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 29 additions & 16 deletions env_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ class EnvSpecSyntaxError(Exception):
Excpetion used for syntax errors.
"""

pass
def __init__(self, message, line):
super().__init__(message)
self.line = line


valid_types_list = [
Expand Down Expand Up @@ -124,50 +126,57 @@ def parse(env_spec_text):
comment_regex = r"^(.+)\#(.+)$"

lines = env_spec_text.split("\n")
enumerated_list = list(enumerate(lines))
Copy link
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
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
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
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
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 😄!


for line in lines:
for line in enumerated_list:
line_number = line[0]
env_spec_line = line[1]
env_spec_entry = {}

line_comment_match = re.match(line_comment_regex, line)
line_comment_match = re.match(line_comment_regex, env_spec_line)

if line_comment_match:
continue

env_spec_entry["comment"] = None
comment_match = re.match(comment_regex, line)
comment_match = re.match(comment_regex, env_spec_line)

if comment_match:
line = comment_match.groups()[0]
env_spec_line = comment_match.groups()[0]
comment = comment_match.groups()[1]
env_spec_entry["comment"] = comment

name_match = re.match(name_regex, line)
name_match = re.match(name_regex, env_spec_line)

if name_match:
name = name_match.groups()[0]
line = name_match.groups()[1]
env_spec_line = name_match.groups()[1]

is_variable_name_valid = re.match(
alphanumeric_that_does_not_start_with_digit, name
)

if not is_variable_name_valid:
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid variable name.")
raise EnvSpecSyntaxError(
"SYNTAX ERROR: Invalid variable name.", line_number
Copy link
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.

)

env_spec_entry["name"] = name
env_spec_entry["choices"] = None
env_spec_entry["default_value"] = None

choices_match = re.match(choices_regex, line)
default_value_match = re.match(default_values_regex, line)
choices_match = re.match(choices_regex, env_spec_line)
default_value_match = re.match(default_values_regex, env_spec_line)

if choices_match:
choices_str = choices_match.groups()[0]

choices = create_list(choices_str)

if not choices:
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid choices list.")
raise EnvSpecSyntaxError(
"SYNTAX ERROR: Invalid choices list.", line_number
Copy link
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.

)

env_spec_entry["choices"] = choices

Expand All @@ -177,32 +186,36 @@ def parse(env_spec_text):

if choices_match:
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
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.

)

env_spec_entry["default_value"] = default_value

if not choices_match:
if default_value_match:
env_spec_type = default_value_match.groups()[0]
else:
env_spec_type = line
env_spec_type = env_spec_line

env_spec_type = env_spec_type.strip()

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
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.


env_spec_entry["type"] = env_spec_type
else:
env_spec_entry["type"] = "text"
else:
name = line.strip()
name = env_spec_line.strip()
is_variable_name_valid = re.match(
alphanumeric_that_does_not_start_with_digit, name
)

if not is_variable_name_valid:
raise EnvSpecSyntaxError("SYNTAX ERROR: Invalid variable name.")
raise EnvSpecSyntaxError(
Copy link
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?

"SYNTAX ERROR: Invalid variable name.", line_number
)

env_spec_entry["name"] = name
env_spec_entry["type"] = "text"
Expand Down