Highlight errors in editor#4467
Conversation
| --editor-searchbar-height: 27px; | ||
| --editor-second-searchbar-height: 27px; | ||
| --debug-line-error-border: rgb(255, 0, 0); | ||
| --debug-expression-error-background: rgba(255, 127, 127, 0.5); |
There was a problem hiding this comment.
Not sure which color to use as background.
This looked ok for me...
There was a problem hiding this comment.
Nice!
The background feels a bit strong to me, maybe lighter, or no background with just the red border lines
There was a problem hiding this comment.
Let's do something light :)
| return; | ||
| } | ||
|
|
||
| // if (pauseInfo && pauseInfo.why.type === "exception") { |
There was a problem hiding this comment.
I forgot to remove this
| }); | ||
| this.setState({ debugExpression }); | ||
| } else { | ||
| doc.addLineClass(line, "line", "new-debug-line"); |
There was a problem hiding this comment.
these two branches are almost exactly the same, with the exception of the the class names. how about something like this:
type TextClasses = {
markTextClass: string,
lineClass: string
}
getTextClasses(pauseInfo: Object): TextClasses {
if (pauseInfo && pauseInfo.why.type === "exception") {
return { markTextClass: "debug-expression-error", lineClass: "debug-expression-error" }
}
return { markTextClass: "debug-expression", lineClass: "new-debug-line" }
}
// ...
const { markTextClass, lineClass } = getTextClasses(pauseInfo);
doc.addLineClass(line, "line", markTextClass);
const debugExpression = markText(editor, lineClass, {
start: { line, column },
end: { line, column: null }
});
this.setState({ debugExpression });this way, it is clear what is changing in the code, its not the entire if block, but just the class names
There was a problem hiding this comment.
You are right. Will change that
wldcordeiro
left a comment
There was a problem hiding this comment.
This is a really nice UX win 👍 Thanks a ton @nyrosmith
Associated Issue: #3112
I closed #4439 because after the editor refactoring I wasn't able to merge/rebase anymore.