fixes #446: enhancements for code clarity and consistency #447
Conversation
| pass | ||
| except SyntaxError as syntax_error: | ||
| print(f"SyntaxError: {syntax_error}") | ||
| raise syntax_error |
There was a problem hiding this comment.
I think both of these changes with SyntaxError should be reverted. This particular line (reraising the error instead of ignoring it) is a major change in behaviour that I don't think is desirable. I also don't think it's a good idea to print in either case, what's the reasoning?
| @@ -60,6 +70,15 @@ def match_returns_stdout(func, solution): | |||
|
|
|||
|
|
|||
| def clean_result(result): | |||
| """ | |||
| Clean the result string for display | |||
There was a problem hiding this comment.
I'm finding that a lot of these docstrings are not very helpful and a reader doesn't learn much from them that they couldn't guess from the function/argument names or see from a quick glance at the code. This is a strong example of that.
| if entry["source"] == "shell": | ||
| return [], "shell" | ||
|
|
||
| messages = input_messages(runner.input_nodes) | ||
| generated_messages = input_messages(runner.input_nodes) |
There was a problem hiding this comment.
I'm not finding this rename helpful.
| @@ -118,7 +118,7 @@ def input_callback(_data=None): | |||
| print(f"<input: {result}>") | |||
| return result | |||
| else: | |||
| raise ValueError(t.Terms.no_more_test_inputs) | |||
| raise ValueError("No more test inputs available.") | |||
There was a problem hiding this comment.
The values in t.Terms change depending on the language.
|
Many changes here (such as deleting service-worker.js) are definitely bad, and the rest are not clearly an improvement. Thank you for taking the time to contribute, but in future I recommend not making so many changes in a single PR in a project that you're not familiar with. |
Fixes #446
Enhancements for code clarity and consistency.