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

[LANGUAGE] improve clear errors #4826

Merged
merged 14 commits into from
Dec 4, 2023
4 changes: 4 additions & 0 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -562,11 +562,15 @@ def parse():

if transpile_result.has_turtle:
response['has_turtle'] = True

if transpile_result.has_clear:
response['has_clear'] = True
except Exception:
pass

with querylog.log_time('detect_sleep'):
Copy link
Member Author

Choose a reason for hiding this comment

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

@rix0rrr I don't think I understand what is happening here, and why, do you?

Firstly, why the exception? an "in" can't really fail, can it?

Secondly what does the logging do here exactly? Why do we need to log the time that this sleep checking occurs (if I understand correctly)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the except is there for safety. This used to be a function call.

The log_time is there to measure how long it takes to do this, in case we need to investigate slowness. With the new code shape, probably not necessary anymore either.

try:
# FH, Nov 2023: hmmm I don't love that this is not done in the same place as the other "has"es
response['has_sleep'] = 'sleep' in transpile_result.commands
except BaseException:
pass
Expand Down
43 changes: 26 additions & 17 deletions hedy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1475,7 +1475,7 @@ def empty_line(self, meta, args):

def forward(self, meta, args):
if len(args) == 0:
return sleep_after(f't.forward(50){self.add_debug_breakpoint()}', False, self.is_debug)
return add_sleep_to_command(f't.forward(50){self.add_debug_breakpoint()}', False, self.is_debug, location="after")
return self.make_forward(int(args[0]))

def color(self, meta, args):
Expand Down Expand Up @@ -1527,7 +1527,7 @@ def make_turtle_command(self, parameter, command, command_text, add_sleep, type)
raise Exception({exception_text})
t.{command_text}(min(600, {variable}) if {variable} > 0 else max(-600, {variable})){self.add_debug_breakpoint()}""")
if add_sleep and not self.is_debug:
return sleep_after(transpiled, False, self.is_debug)
return add_sleep_to_command(transpiled, False, self.is_debug, location="after")
return transpiled

def make_turtle_color_command(self, parameter, command, command_text, language):
Expand Down Expand Up @@ -1647,7 +1647,7 @@ def ask(self, meta, args):

def forward(self, meta, args):
if len(args) == 0:
return sleep_after(f't.forward(50){self.add_debug_breakpoint()}', False, self.is_debug)
return add_sleep_to_command(f't.forward(50){self.add_debug_breakpoint()}', False, self.is_debug, location="after")

if ConvertToPython.is_int(args[0]):
parameter = int(args[0])
Expand Down Expand Up @@ -1780,8 +1780,14 @@ def ask(self, meta, args):
def error_print_nq(self, meta, args):
return ConvertToPython_2.print(self, meta, args)

def clear(self, meta, args): # todo not sure about it being here
return f"""extensions.clear(){self.add_debug_breakpoint()}
def clear(self, meta, args):
command = "extensions.clear()"

# add two sleeps, one is a bit brief
command = add_sleep_to_command(command, False, self.is_debug, "before")
command = add_sleep_to_command(command, False, self.is_debug, "before")

return f"""{command}{self.add_debug_breakpoint()}
try:
# If turtle is being used, reset canvas
t.hideturtle()
Expand Down Expand Up @@ -2002,7 +2008,7 @@ def turn(self, meta, args):

def forward(self, meta, args):
if len(args) == 0:
return sleep_after('t.forward(50)' + self.add_debug_breakpoint(), False, self.is_debug)
return add_sleep_to_command('t.forward(50)' + self.add_debug_breakpoint(), False, self.is_debug, location="after")
arg = args[0]
if self.is_variable(arg):
return self.make_forward(escape_var(arg))
Expand All @@ -2011,7 +2017,7 @@ def forward(self, meta, args):
return self.make_forward(int(args[0]))


def sleep_after(commands, indent=True, is_debug=False):
def add_sleep_to_command(commands, indent=True, is_debug=False, location="after"):
if is_debug:
return commands

Expand All @@ -2020,7 +2026,10 @@ def sleep_after(commands, indent=True, is_debug=False):
return commands

sleep_command = "time.sleep(0.1)" if indent is False else " time.sleep(0.1)"
return commands + "\n" + sleep_command
if location == "after":
return commands + "\n" + sleep_command
else: # location is before
return sleep_command + "\n" + commands


@v_args(meta=True)
Expand All @@ -2032,7 +2041,7 @@ def repeat(self, meta, args):
times = self.process_variable(args[0], meta.line)
command = args[1]
# in level 7, repeats can only have 1 line as their arguments
command = sleep_after(command, False, self.is_debug)
command = add_sleep_to_command(command, False, self.is_debug, location="after")
return f"""for {var_name} in range(int({str(times)})):{self.add_debug_breakpoint()}
{ConvertToPython.indent(command)}"""

Expand All @@ -2056,7 +2065,7 @@ def repeat(self, meta, args):

all_lines = [ConvertToPython.indent(x) for x in args[1:]]
body = "\n".join(all_lines)
body = sleep_after(body, indent=True, is_debug=self.is_debug)
body = add_sleep_to_command(body, indent=True, is_debug=self.is_debug, location="after")

return f"for {var_name} in range(int({times})):{self.add_debug_breakpoint()}\n{body}"

Expand Down Expand Up @@ -2144,7 +2153,7 @@ def for_list(self, meta, args):

body = "\n".join([ConvertToPython.indent(x) for x in args[2:]])

body = sleep_after(body, True, self.is_debug)
body = add_sleep_to_command(body, True, self.is_debug, location="after")
return f"for {times} in {args[1]}:{ self.add_debug_breakpoint() }\n{body}"


Expand All @@ -2156,7 +2165,7 @@ def for_loop(self, meta, args):
args = [a for a in args if a != ""] # filter out in|dedent tokens
iterator = escape_var(args[0])
body = "\n".join([ConvertToPython.indent(x) for x in args[3:]])
body = sleep_after(body, True, self.is_debug)
body = add_sleep_to_command(body, True, self.is_debug, location="after")
stepvar_name = self.get_fresh_var('step')
begin = self.process_token_or_tree(args[1])
end = self.process_token_or_tree(args[2])
Expand Down Expand Up @@ -2299,7 +2308,7 @@ def turn(self, meta, args):

def forward(self, meta, args):
if len(args) == 0:
return sleep_after('t.forward(50)' + self.add_debug_breakpoint(), False, self.is_debug)
return add_sleep_to_command('t.forward(50)' + self.add_debug_breakpoint(), False, self.is_debug, location="after")
arg = args[0]
if self.is_variable(arg):
return self.make_forward(escape_var(arg))
Expand Down Expand Up @@ -2371,7 +2380,7 @@ def while_loop(self, meta, args):
args = [a for a in args if a != ""] # filter out in|dedent tokens
all_lines = [ConvertToPython.indent(x) for x in args[1:]]
body = "\n".join(all_lines)
body = sleep_after(body, True, self.is_debug)
body = add_sleep_to_command(body, True, self.is_debug, location="after")
exceptions = self.make_catch_exception([args[0]])
return exceptions + "while " + args[0] + ":" + self.add_debug_breakpoint() + "\n" + body

Expand Down Expand Up @@ -2852,7 +2861,7 @@ def get_parser(level, lang="en", keep_all_tokens=False, skip_faulty=False):
return lark


ParseResult = namedtuple('ParseResult', ['code', 'source_map', 'has_turtle', 'has_pygame', 'commands'])
ParseResult = namedtuple('ParseResult', ['code', 'source_map', 'has_turtle', 'has_pygame', 'has_clear', 'commands'])


def transpile_inner_with_skipping_faulty(input_string, level, lang="en"):
Expand Down Expand Up @@ -3278,7 +3287,6 @@ def is_program_valid(program_root, input_string, level, lang):
line = invalid_info.line
column = invalid_info.column
if invalid_info.error_type == ' ':

# the error here is a space at the beginning of a line, we can fix that!
fixed_code = program_repair.remove_leading_spaces(input_string)
if fixed_code != input_string: # only if we have made a successful fix
Expand Down Expand Up @@ -3423,13 +3431,14 @@ def transpile_inner(input_string, level, lang="en", populate_source_map=False, i

commands = AllCommands(level).transform(program_root)

has_clear = "clear" in commands
has_turtle = "forward" in commands or "turn" in commands or "color" in commands
has_pygame = "ifpressed" in commands or "ifpressed_else" in commands or "assign_button" in commands

if populate_source_map:
source_map.set_python_output(python)

return ParseResult(python, source_map, has_turtle, has_pygame, commands)
return ParseResult(python, source_map, has_turtle, has_pygame, has_clear, commands)
except VisitError as E:
if isinstance(E, VisitError):
# Exceptions raised inside visitors are wrapped inside VisitError. Unwrap it if it is a
Expand Down
11 changes: 6 additions & 5 deletions static/js/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ export async function runit(level: number, lang: string, disabled_prompt: string
program_data = theGlobalDebugger.get_program_data();
}

runPythonProgram(program_data.Code, program_data.source_map, program_data.has_turtle, program_data.has_pygame, program_data.has_sleep, program_data.Warning, cb, run_type).catch(function(err: any) {
runPythonProgram(program_data.Code, program_data.source_map, program_data.has_turtle, program_data.has_pygame, program_data.has_sleep, program_data.has_clear, program_data.Warning, cb, run_type).catch(function(err: any) {
// The err is null if we don't understand it -> don't show anything
if (err != null) {
error.show(ClientMessages['Execute_error'], err.message);
Expand Down Expand Up @@ -827,7 +827,7 @@ window.onerror = function reportClientException(message, source, line_number, co
});
}

export function runPythonProgram(this: any, code: string, sourceMap: any, hasTurtle: boolean, hasPygame: boolean, hasSleep: boolean, hasWarnings: boolean, cb: () => void, run_type: "run" | "debug" | "continue") {
export function runPythonProgram(this: any, code: string, sourceMap: any, hasTurtle: boolean, hasPygame: boolean, hasSleep: boolean, hasClear: boolean, hasWarnings: boolean, cb: () => void, run_type: "run" | "debug" | "continue") {
// If we are in the Parsons problem -> use a different output
let outputDiv = $('#output');
let skip_faulty_found_errors = false;
Expand Down Expand Up @@ -1034,13 +1034,13 @@ export function runPythonProgram(this: any, code: string, sourceMap: any, hasTur
}

// Check if the program was correct but the output window is empty: Return a warning
if ($('#output').is(':empty') && $('#turtlecanvas').is(':empty')) {
if ((!hasClear) && $('#output').is(':empty') && $('#turtlecanvas').is(':empty')) {
pushAchievement("error_or_empty");
error.showWarning(ClientMessages['Transpile_warning'], ClientMessages['Empty_output']);
return;
}
if (!hasWarnings && code !== last_code) {
showSuccesMessage();
showSuccesMessage(); //FH nov 2023: typo in success :)
last_code = code;
}
if (cb) cb ();
Expand Down Expand Up @@ -1088,7 +1088,8 @@ export function runPythonProgram(this: any, code: string, sourceMap: any, hasTur
Code: code,
source_map: sourceMap,
has_turtle: hasTurtle,
has_pygame: hasPygame,
has_pygame: hasPygame, //here too: where is hassleep?
has_clear: hasClear,
Warning: hasWarnings
});

Expand Down
7 changes: 6 additions & 1 deletion tests/test_level/test_level_04.py
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,8 @@ def test_lonely_text(self):
def test_clear(self):
code = "clear"
expected = textwrap.dedent("""\
time.sleep(0.1)
time.sleep(0.1)
extensions.clear()
try:
# If turtle is being used, reset canvas
Expand All @@ -925,7 +927,10 @@ def test_clear(self):
except NameError:
pass""")

self.multi_level_tester(code=code, expected=expected)
self.multi_level_tester(code=code,
expected=expected,
extra_check_function=(lambda result: result.has_clear)
)

def test_source_map(self):
code = textwrap.dedent("""\
Expand Down
Loading