Skip to content

Commit

Permalink
fix: higlight error location in line (hedyorg#1184)
Browse files Browse the repository at this point in the history
This is a resubmission of hedyorg#1147, which had done most of the heavy lifting
to figure out what needed to happen already but had some unnecessary
changes in there as well.

The `Location` field in the error response already contained both row
and column number, so good enough to achieve column-specific
highlighting.

To support `InvalidCommandException`, which *can* only have a row and
not a column (since we don't have that information available), we
introduce the convention that `Location` can be either `[row, col]` or
just `[row]`. The client will highlight the entire line or just a single
character.

(I added a helper future-proofed a bit by adding a parameter so that
once we want to highlight whole words, we can do that too).

There is a bit of a short-term hack in `InvalidCommandException`,
which should be tackled as part of addressing hedyorg#1178.

Co-authored-by: Felienne <[email protected]>
  • Loading branch information
rix0rrr and Felienne authored Nov 6, 2021
1 parent b477b86 commit 3763e13
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 30 deletions.
12 changes: 8 additions & 4 deletions hedy.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,12 @@ def __init__(self, **arguments):
super().__init__('Too Big', **arguments)

class InvalidCommandException(HedyException):
def __init__(self, **arguments):
super().__init__('Invalid', **arguments)
def __init__(self, line_number, **arguments):
super().__init__('Invalid', line_number=line_number, **arguments)

# Location is copied here so that 'hedy_error_to_response' will find it
# Location can be either [row, col] or just [row]
self.location = [line_number]

class IncompleteCommandException(HedyException):
def __init__(self, **arguments):
Expand Down Expand Up @@ -1701,7 +1705,7 @@ def transpile_inner(input_string, level):
invalid_argument=''.join(invalid_info.arguments))
# clearly the error message here should be better or it should be a different one!
raise ParseException(level=level, location=["?", "?"], keyword_found=invalid_command)
raise InvalidCommandException(invalid_command=invalid_command, level=level, guessed_command=closest)
raise InvalidCommandException(invalid_command=invalid_command, level=level, guessed_command=closest, line_number=line)

is_complete = IsComplete(level).transform(program_root)
if not is_complete[0]:
Expand Down Expand Up @@ -1732,7 +1736,7 @@ def transpile_inner(input_string, level):
return ParseResult(python, has_turtle)

def execute(input_string, level):
python = transpile(input_string, level)
python = transpile(input_string, level)
if python.has_turtle:
raise HedyException("hedy.execute doesn't support turtle")
exec(python.code)
Expand Down
67 changes: 48 additions & 19 deletions static/js/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,32 +211,17 @@ export function runit(level: string, lang: string, cb: () => void) {
}),
contentType: 'application/json',
dataType: 'json'
}).done(function(response) {
}).done(function(response: any) {
console.log('Response', response);
if (response.Warning) {
error.showWarning(ErrorMessages['Transpile_warning'], response.Warning);
}
if (response.Error) {
error.show(ErrorMessages['Transpile_error'], response.Error);
if (response.Location && response.Location[0] != "?") {
editor.session.setAnnotations([
{
row: response.Location[0] - 1,
column: response.Location[1] - 1,
text: "",
type: "error",
}
]);
// FIXME change this to apply only to the error span once errors have an end location.
editor.session.addMarker(
new ace.Range(
response.Location[0] - 1,
response.Location[1] - 1,
response.Location[0] - 1,
response.Location[1],
),
"editor-error", "fullLine", false
);
// Location can be either [row, col] or just [row].

highlightAceError(editor, response.Location[0], response.Location[1]);
}
return;
}
Expand All @@ -261,6 +246,50 @@ export function runit(level: string, lang: string, cb: () => void) {
}
}

/**
* Mark an error location in the ace editor
*
* The error occurs at the given row, and optionally has a column and
* and a length.
*
* If 'col' is not given, the entire line will be highlighted red. Otherwise
* the character at 'col' will be highlighted, optionally extending for
* 'length' characters.
*
* 'row' and 'col' are 1-based.
*/
function highlightAceError(editor: AceAjax.Editor, row: number, col?: number, length=1) {
// This adds a red cross in the left margin.
// Not sure what the "column" argument does here -- it doesn't seem
// to make a difference.
editor.session.setAnnotations([
{
row: row - 1,
column: (col ?? 1) - 1,
text: '',
type: 'error',
}
]);

if (col === undefined) {
// Higlight entire row
editor.session.addMarker(
new ace.Range(row - 1, 1, row - 1, 2),
"editor-error", "fullLine", false
);
return;
}

// Highlight span
editor.session.addMarker(
new ace.Range(
row - 1, col - 1,
row - 1, col - 1 + length,
),
"editor-error", "text", false
);
}

/**
* Called when the user clicks the "Try" button in one of the palette buttons
*/
Expand Down
Loading

0 comments on commit 3763e13

Please sign in to comment.