From 48418e5732e1b1e2a10207c8007a5f959e422f20 Mon Sep 17 00:00:00 2001 From: Yury Smolsky Date: Mon, 16 Apr 2018 19:40:48 +0300 Subject: [PATCH] godoc: display "go vet" errors before the output of a program This change adds an option to run "go vet" for the playground program and display errors before any output. To enable this, the playground function has to be supplied with opts.enableVet set to true. Vet check is performed only for succesfully run programs, meaning that the "/compile" endpoint returned no errors. This change highlights lines printed to stderr as errors (in red). There is a corresponding change for the Playground: CL 100776. Updates golang/go#7597 Updates golang/go#24576 Change-Id: I8c0f8c1189c461338b5bce57777b12aecab268fb Reviewed-on: https://go-review.googlesource.com/107455 Run-TryBot: Yury Smolsky TryBot-Result: Gobot Gobot Reviewed-by: Andrew Bonventre --- godoc/static/playground.js | 74 ++++++++++++++++++-------------------- godoc/static/static.go | 74 ++++++++++++++++++-------------------- 2 files changed, 70 insertions(+), 78 deletions(-) diff --git a/godoc/static/playground.js b/godoc/static/playground.js index 85c43556..aad459fa 100644 --- a/godoc/static/playground.js +++ b/godoc/static/playground.js @@ -40,11 +40,12 @@ here's a skeleton implementation of a playground transport. } */ -function HTTPTransport() { +// HTTPTransport is the default transport. +// enableVet enables running vet if a program was compiled and ran successfully. +// If vet returned any errors, display them before the output of a program. +function HTTPTransport(enableVet) { 'use strict'; - // TODO(adg): support stderr - function playback(output, events) { var timeout; output({Kind: 'start'}); @@ -55,12 +56,12 @@ function HTTPTransport() { } var e = events.shift(); if (e.Delay === 0) { - output({Kind: 'stdout', Body: e.Message}); + output({Kind: e.Kind, Body: e.Message}); next(); return; } timeout = setTimeout(function() { - output({Kind: 'stdout', Body: e.Message}); + output({Kind: e.Kind, Body: e.Message}); next(); }, e.Delay / 1000000); } @@ -69,7 +70,7 @@ function HTTPTransport() { Stop: function() { clearTimeout(timeout); } - } + }; } function error(output, msg) { @@ -96,7 +97,28 @@ function HTTPTransport() { error(output, data.Errors); return; } - playing = playback(output, data.Events); + + if (!enableVet) { + playing = playback(output, data.Events); + return; + } + + $.ajax("/vet", { + data: {"body": body}, + type: "POST", + dataType: "json", + success: function(dataVet) { + if (dataVet.Errors) { + // inject errors from the vet as the first event in the output + data.Events.unshift({Message: 'Go vet exited.\n\n', Kind: 'system', Delay: 0}); + data.Events.unshift({Message: dataVet.Errors, Kind: 'stderr', Delay: 0}); + } + playing = playback(output, data.Events); + }, + error: function() { + playing = playback(output, data.Events); + } + }); }, error: function() { error(output, 'Error communicating with remote server.'); @@ -127,7 +149,7 @@ function SocketTransport() { websocket.onclose = function() { console.log('websocket connection closed'); - } + }; websocket.onmessage = function(e) { var m = JSON.parse(e.data); @@ -139,7 +161,7 @@ function SocketTransport() { started[m.Id] = true; } output({Kind: m.Kind, Body: m.Body}); - } + }; function send(m) { websocket.send(JSON.stringify(m)); @@ -207,7 +229,7 @@ function PlaygroundOutput(el) { if (needScroll) el.scrollTop = el.scrollHeight - el.offsetHeight; - } + }; } (function() { @@ -223,7 +245,7 @@ function PlaygroundOutput(el) { return function(write) { if (write.Body) lineHighlight(write.Body); wrappedOutput(write); - } + }; } function lineClear() { $(".lineerror").removeClass("lineerror"); @@ -238,14 +260,14 @@ function PlaygroundOutput(el) { // shareEl - share button element (optional) // shareURLEl - share URL text input element (optional) // shareRedirect - base URL to redirect to on share (optional) - // vetEl - vet button element (optional) // toysEl - toys select element (optional) // enableHistory - enable using HTML5 history API (optional) // transport - playground transport to use (default is HTTPTransport) // enableShortcuts - whether to enable shortcuts (Ctrl+S/Cmd+S to save) (default is false) + // enableVet - enable running vet and displaying its errors function playground(opts) { var code = $(opts.codeEl); - var transport = opts['transport'] || new HTTPTransport(); + var transport = opts['transport'] || new HTTPTransport(opts['enableVet']); var running; // autoindent helpers. @@ -369,11 +391,6 @@ function PlaygroundOutput(el) { if (running) running.Kill(); output.removeClass("error").text('Waiting for remote server...'); } - function noError() { - lineClear(); - if (running) running.Kill(); - output.removeClass("error").text('No errors.'); - } function run() { loading(); running = transport.Run(body(), highlightOutput(PlaygroundOutput(output[0]))); @@ -400,26 +417,6 @@ function PlaygroundOutput(el) { }); } - function vet() { - loading(); - var data = {"body": body()}; - $.ajax("/vet", { - data: data, - type: "POST", - dataType: "json", - success: function(data) { - if (data.Errors) { - setError(data.Errors); - } else { - noError(); - } - }, - error: function() { - setError('Error communicating with remote server.'); - } - }); - } - var shareURL; // jQuery element to show the shared URL. var sharing = false; // true if there is a pending request. var shareCallbacks = []; @@ -467,7 +464,6 @@ function PlaygroundOutput(el) { $(opts.runEl).click(run); $(opts.fmtEl).click(fmt); - $(opts.vetEl).click(vet); if (opts.shareEl !== null && (opts.shareURLEl !== null || opts.shareRedirect !== null)) { if (opts.shareURLEl) { diff --git a/godoc/static/static.go b/godoc/static/static.go index 22351f2f..05117621 100644 --- a/godoc/static/static.go +++ b/godoc/static/static.go @@ -2329,11 +2329,12 @@ here's a skeleton implementation of a playground transport. } */ -function HTTPTransport() { +// HTTPTransport is the default transport. +// enableVet enables running vet if a program was compiled and ran successfully. +// If vet returned any errors, display them before the output of a program. +function HTTPTransport(enableVet) { 'use strict'; - // TODO(adg): support stderr - function playback(output, events) { var timeout; output({Kind: 'start'}); @@ -2344,12 +2345,12 @@ function HTTPTransport() { } var e = events.shift(); if (e.Delay === 0) { - output({Kind: 'stdout', Body: e.Message}); + output({Kind: e.Kind, Body: e.Message}); next(); return; } timeout = setTimeout(function() { - output({Kind: 'stdout', Body: e.Message}); + output({Kind: e.Kind, Body: e.Message}); next(); }, e.Delay / 1000000); } @@ -2358,7 +2359,7 @@ function HTTPTransport() { Stop: function() { clearTimeout(timeout); } - } + }; } function error(output, msg) { @@ -2385,7 +2386,28 @@ function HTTPTransport() { error(output, data.Errors); return; } - playing = playback(output, data.Events); + + if (!enableVet) { + playing = playback(output, data.Events); + return; + } + + $.ajax("/vet", { + data: {"body": body}, + type: "POST", + dataType: "json", + success: function(dataVet) { + if (dataVet.Errors) { + // inject errors from the vet as the first event in the output + data.Events.unshift({Message: 'Go vet exited.\n\n', Kind: 'system', Delay: 0}); + data.Events.unshift({Message: dataVet.Errors, Kind: 'stderr', Delay: 0}); + } + playing = playback(output, data.Events); + }, + error: function() { + playing = playback(output, data.Events); + } + }); }, error: function() { error(output, 'Error communicating with remote server.'); @@ -2416,7 +2438,7 @@ function SocketTransport() { websocket.onclose = function() { console.log('websocket connection closed'); - } + }; websocket.onmessage = function(e) { var m = JSON.parse(e.data); @@ -2428,7 +2450,7 @@ function SocketTransport() { started[m.Id] = true; } output({Kind: m.Kind, Body: m.Body}); - } + }; function send(m) { websocket.send(JSON.stringify(m)); @@ -2496,7 +2518,7 @@ function PlaygroundOutput(el) { if (needScroll) el.scrollTop = el.scrollHeight - el.offsetHeight; - } + }; } (function() { @@ -2512,7 +2534,7 @@ function PlaygroundOutput(el) { return function(write) { if (write.Body) lineHighlight(write.Body); wrappedOutput(write); - } + }; } function lineClear() { $(".lineerror").removeClass("lineerror"); @@ -2527,14 +2549,14 @@ function PlaygroundOutput(el) { // shareEl - share button element (optional) // shareURLEl - share URL text input element (optional) // shareRedirect - base URL to redirect to on share (optional) - // vetEl - vet button element (optional) // toysEl - toys select element (optional) // enableHistory - enable using HTML5 history API (optional) // transport - playground transport to use (default is HTTPTransport) // enableShortcuts - whether to enable shortcuts (Ctrl+S/Cmd+S to save) (default is false) + // enableVet - enable running vet and displaying its errors function playground(opts) { var code = $(opts.codeEl); - var transport = opts['transport'] || new HTTPTransport(); + var transport = opts['transport'] || new HTTPTransport(opts['enableVet']); var running; // autoindent helpers. @@ -2658,11 +2680,6 @@ function PlaygroundOutput(el) { if (running) running.Kill(); output.removeClass("error").text('Waiting for remote server...'); } - function noError() { - lineClear(); - if (running) running.Kill(); - output.removeClass("error").text('No errors.'); - } function run() { loading(); running = transport.Run(body(), highlightOutput(PlaygroundOutput(output[0]))); @@ -2689,26 +2706,6 @@ function PlaygroundOutput(el) { }); } - function vet() { - loading(); - var data = {"body": body()}; - $.ajax("/vet", { - data: data, - type: "POST", - dataType: "json", - success: function(data) { - if (data.Errors) { - setError(data.Errors); - } else { - noError(); - } - }, - error: function() { - setError('Error communicating with remote server.'); - } - }); - } - var shareURL; // jQuery element to show the shared URL. var sharing = false; // true if there is a pending request. var shareCallbacks = []; @@ -2756,7 +2753,6 @@ function PlaygroundOutput(el) { $(opts.runEl).click(run); $(opts.fmtEl).click(fmt); - $(opts.vetEl).click(vet); if (opts.shareEl !== null && (opts.shareURLEl !== null || opts.shareRedirect !== null)) { if (opts.shareURLEl) {