From 0a2875c880d11b1545a78aebe56b85c5c43c4ead Mon Sep 17 00:00:00 2001 From: Phoenix Eliot Date: Wed, 24 Aug 2016 17:09:04 -0700 Subject: [PATCH] Add functional error banners (with poor messages) --- app/schemas/subscriptions/tome.coffee | 2 +- app/views/play/level/tome/Problem.coffee | 79 ++++++++++++++----- .../play/level/tome/ProblemAlertView.coffee | 16 ++-- app/views/play/level/tome/SpellView.coffee | 39 ++++++--- server/commons/database.coffee | 1 - 5 files changed, 94 insertions(+), 43 deletions(-) diff --git a/app/schemas/subscriptions/tome.coffee b/app/schemas/subscriptions/tome.coffee index cff846e0c..e1e97b576 100644 --- a/app/schemas/subscriptions/tome.coffee +++ b/app/schemas/subscriptions/tome.coffee @@ -89,7 +89,7 @@ module.exports = 'tome:problems-updated': c.object {title: 'Problems Updated', description: 'Published when problems have been updated', required: ['spell', 'problems', 'isCast']}, spell: {type: 'object'} problems: {type: 'array'} - isCast: {type: 'boolean'} + isCast: {type: 'boolean'} # Whether the code has been Run yet. Sometimes determines if error displays as just annotation or as full banner. 'tome:change-language': c.object {title: 'Tome Change Language', description: 'Published when the Tome should update its programming language', required: ['language']}, language: {type: 'string'} diff --git a/app/views/play/level/tome/Problem.coffee b/app/views/play/level/tome/Problem.coffee index 9b5b22fa8..3071bbc99 100644 --- a/app/views/play/level/tome/Problem.coffee +++ b/app/views/play/level/tome/Problem.coffee @@ -1,43 +1,82 @@ Range = ace.require('ace/range').Range +# This class can either wrap an AetherProblem, +# or act as a general runtime error container for web-dev iFrame errors. module.exports = class Problem annotation: null markerRange: null - constructor: (@aether, @aetherProblem, @ace, isCast=false, @levelID) -> - @buildAnnotation() - @buildMarkerRange() if isCast + # TODO: Convert calls to constructor to use object + constructor: ({ @aether, @aetherProblem, @ace, isCast=false, @levelID, error }) -> + debugger if arguments.length > 1 + if @aetherProblem + @annotation = @buildAnnotationFromAetherProblem(@aetherProblem) + {@lineMarkerRange, @textMarkerRange} = @buildMarkerRangesFromAetherProblem(@aetherProblem) if isCast + + @level = @aetherProblem.level + @row = @aetherProblem.range?[0]?.row + @column = @aetherProblem.range?[0]?.col + @range = @aetherProblem.range + @message = @aetherProblem.message + @hint = @aetherProblem.hint + @userInfo = @aetherProblem.userInfo + else + @annotation = @buildAnnotationFromWebDevError(error) + @lineMarkerRange = new Range error.line, 0, error.line, 1 + @lineMarkerRange.start = @ace.getSession().getDocument().createAnchor @lineMarkerRange.start + @lineMarkerRange.end = @ace.getSession().getDocument().createAnchor @lineMarkerRange.end + @lineMarkerRange.id = @ace.getSession().addMarker @lineMarkerRange, 'problem-line', 'fullLine' + + @level = error?.type or 'error' + @row = error?.line + @column = error?.column + @message = error.text or error.raw or 'Unknown Error' + @hint = undefined + @userInfo = undefined + # TODO: get ACE screen line, too, for positioning, since any multiline "lines" will mess up positioning Backbone.Mediator.publish("problem:problem-created", line: @annotation.row, text: @annotation.text) if application.isIPadApp destroy: -> @removeMarkerRanges() @userCodeProblem.off() if @userCodeProblem + + buildAnnotationFromWebDevError: (error) -> + { + row: error.line + column: error.column + raw: error.error + text: error.message + type: error.type + createdBy: 'web-dev-iframe' + } - buildAnnotation: -> - return unless @aetherProblem.range - text = @aetherProblem.message.replace /^Line \d+: /, '' - start = @aetherProblem.range[0] - @annotation = + buildAnnotationFromAetherProblem: (aetherProblem) -> + return unless aetherProblem.range + text = aetherProblem.message.replace /^Line \d+: /, '' + start = aetherProblem.range[0] + { row: start.row, column: start.col, raw: text, text: text, type: @aetherProblem.level ? 'error' createdBy: 'aether' + } - buildMarkerRange: -> - return unless @aetherProblem.range - [start, end] = @aetherProblem.range - textClazz = "problem-marker-#{@aetherProblem.level}" - @textMarkerRange = new Range start.row, start.col, end.row, end.col - @textMarkerRange.start = @ace.getSession().getDocument().createAnchor @textMarkerRange.start - @textMarkerRange.end = @ace.getSession().getDocument().createAnchor @textMarkerRange.end - @textMarkerRange.id = @ace.getSession().addMarker @textMarkerRange, textClazz, 'text' + buildMarkerRangesFromAetherProblem: (aetherProblem) -> + return unless aetherProblem.range + [start, end] = aetherProblem.range + textClazz = "problem-marker-#{aetherProblem.level}" + textMarkerRange = new Range start.row, start.col, end.row, end.col + textMarkerRange.start = @ace.getSession().getDocument().createAnchor textMarkerRange.start + textMarkerRange.end = @ace.getSession().getDocument().createAnchor textMarkerRange.end + textMarkerRange.id = @ace.getSession().addMarker textMarkerRange, textClazz, 'text' lineClazz = "problem-line" - @lineMarkerRange = new Range start.row, start.col, end.row, end.col - @lineMarkerRange.start = @ace.getSession().getDocument().createAnchor @lineMarkerRange.start - @lineMarkerRange.end = @ace.getSession().getDocument().createAnchor @lineMarkerRange.end - @lineMarkerRange.id = @ace.getSession().addMarker @lineMarkerRange, lineClazz, 'fullLine' + lineMarkerRange = new Range start.row, start.col, end.row, end.col + lineMarkerRange.start = @ace.getSession().getDocument().createAnchor lineMarkerRange.start + lineMarkerRange.end = @ace.getSession().getDocument().createAnchor lineMarkerRange.end + lineMarkerRange.id = @ace.getSession().addMarker lineMarkerRange, lineClazz, 'fullLine' + { lineMarkerRange, textMarkerRange } removeMarkerRanges: -> if @textMarkerRange diff --git a/app/views/play/level/tome/ProblemAlertView.coffee b/app/views/play/level/tome/ProblemAlertView.coffee index c9ffc1363..ae481b1b2 100644 --- a/app/views/play/level/tome/ProblemAlertView.coffee +++ b/app/views/play/level/tome/ProblemAlertView.coffee @@ -38,31 +38,31 @@ module.exports = class ProblemAlertView extends CocoView afterRender: -> super() if @problem? - @$el.addClass('alert').addClass("alert-#{@problem.aetherProblem.level}").hide().fadeIn('slow') - @$el.addClass('no-hint') unless @problem.aetherProblem.hint + @$el.addClass('alert').addClass("alert-#{@problem.level}").hide().fadeIn('slow') + @$el.addClass('no-hint') unless @problem.hint @playSound 'error_appear' setProblemMessage: -> if @problem? format = (s) -> marked(s.replace(//g, '>')) if s? - message = @problem.aetherProblem.message + message = @problem.message # Add time to problem message if hint is for a missing null check # NOTE: This may need to be updated with Aether error hint changes - if @problem.aetherProblem.hint? and /(?:null|undefined)/.test @problem.aetherProblem.hint - age = @problem.aetherProblem.userInfo?.age + if @problem.hint? and /(?:null|undefined)/.test @problem.hint + age = @problem.userInfo?.age if age? if /^Line \d+:/.test message message = message.replace /^(Line \d+)/, "$1, time #{age.toFixed(1)}" else message = "Time #{age.toFixed(1)}: #{message}" @message = format message - @hint = format @problem.aetherProblem.hint + @hint = format @problem.hint onShowProblemAlert: (data) -> return unless $('#code-area').is(":visible") if @problem? - if @$el.hasClass "alert-#{@problem.aetherProblem.level}" - @$el.removeClass "alert-#{@problem.aetherProblem.level}" + if @$el.hasClass "alert-#{@problem.level}" + @$el.removeClass "alert-#{@problem.level}" if @$el.hasClass "no-hint" @$el.removeClass "no-hint" @problem = data.problem diff --git a/app/views/play/level/tome/SpellView.coffee b/app/views/play/level/tome/SpellView.coffee index 04e676936..3f7355b7e 100644 --- a/app/views/play/level/tome/SpellView.coffee +++ b/app/views/play/level/tome/SpellView.coffee @@ -831,11 +831,11 @@ module.exports = class SpellView extends CocoView for aetherProblem, problemIndex in aether.getAllProblems() continue if key = aetherProblem.userInfo?.key and key of seenProblemKeys seenProblemKeys[key] = true if key - @problems.push problem = new Problem aether, aetherProblem, @ace, isCast, @options.levelID + @problems.push problem = new Problem { aether, aetherProblem, @ace, isCast, levelID: @options.levelID } if isCast and problemIndex is 0 - if problem.aetherProblem.range? + if problem.range? lineOffsetPx = 0 - for i in [0...problem.aetherProblem.range[0].row] + for i in [0...problem.row] lineOffsetPx += @aceSession.getRowLength(i) * @ace.renderer.lineHeight lineOffsetPx -= @ace.session.getScrollTop() Backbone.Mediator.publish 'tome:show-problem-alert', problem: problem, lineOffsetPx: Math.max lineOffsetPx, 0 @@ -993,19 +993,32 @@ module.exports = class SpellView extends CocoView @spell.transpile() # TODO: is there any way we can avoid doing this if it hasn't changed? Causes a slight hang. @updateAether false, false - onWebDevError: (e) -> + onWebDevError: (error) -> annotations = @aceSession.getAnnotations() console.log {spellviewOnWebDevError: arguments} - newAnnotation = { - createdBy: 'web-dev-iframe' - row: e.line + @linesBeforeScript(@getSource()) - column: e.column - raw: e.error - text: e.message - type: e.type - } - annotations.push(newAnnotation) + offsetError = _.merge {}, error, { line: error.line + @linesBeforeScript(@getSource()) } + console.log {error, offsetError} + problem = new Problem({ error: offsetError, @ace, levelID: @options.levelID }) + @problems.push problem + + problemIndex = 0 # TODO: Unstub + if problemIndex is 0 + lineOffsetPx = 0 + for i in [0...problem.row] + lineOffsetPx += @aceSession.getRowLength(i) * @ace.renderer.lineHeight + lineOffsetPx -= @ace.session.getScrollTop() + Backbone.Mediator.publish 'tome:show-problem-alert', problem: problem, lineOffsetPx: Math.max lineOffsetPx, 0 + + # @saveUserCodeProblem(aether, aetherProblem) + console.log {annotation: problem.annotation} + annotations.push problem.annotation if problem.annotation @aceSession.setAnnotations annotations, true + Backbone.Mediator.publish 'tome:problems-updated', spell: @spell, problems: @problems, isCast: true + + # TODO: Unset this on rerun + @ace.setStyle 'user-code-problem' + # TODO: When is this unset? + @ace.setStyle 'spell-cast' linesBeforeScript: (html) -> # TODO: refactor, make it work with multiple scripts. What to do when error is in level-creator's code? diff --git a/server/commons/database.coffee b/server/commons/database.coffee index 218ec51f9..ceea28b6a 100644 --- a/server/commons/database.coffee +++ b/server/commons/database.coffee @@ -206,4 +206,3 @@ module.exports = return false if index is -1 return false if delta.deltaPath[index+1] in ['en', 'en-US', 'en-GB'] # English speakers are most likely just spamming, so always treat those as patches, not saves. return true -