From 6fd02abd9d5a10613a7433cab012b30dd9f5d8ea Mon Sep 17 00:00:00 2001 From: Noah Santschi-Cooney Date: Sun, 5 Aug 2018 00:10:40 +0100 Subject: [PATCH] Fixed errors still propogating of the include was removed from the parent. Also supressing 'shaderpacks' path not found --- README.md | 1 + server/src/config.ts | 13 +++++++++-- server/src/graph.ts | 2 +- server/src/linter.ts | 51 +++++++++++++++++++++++++++----------------- server/src/server.ts | 8 +++++-- server/src/utils.ts | 3 ++- 6 files changed, 52 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index e389d71..88e2aa2 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ VSCode-MC-Shader is a [Visual Studio Code](https://code.visualstudio.com/) exten ## Planned +- Multi-workspaces (currently only one is supported and using multiple is very undefined behaviour) - Warnings for unused uniforms/varyings - Some cool `DRAWBUFFERS` stuff diff --git a/server/src/config.ts b/server/src/config.ts index 1154e2c..206f85c 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -7,6 +7,7 @@ import { postError } from './utils' import { execSync } from 'child_process' import { serverLog } from './logging' import { dirname } from 'path' +import { DidChangeConfigurationParams } from 'vscode-languageserver/lib/main' const url = { 'win32': 'https://github.com/KhronosGroup/glslang/releases/download/master-tot/glslang-master-windows-x64-Release.zip', @@ -23,7 +24,9 @@ export interface Config { export let conf: Config = {shaderpacksPath: '', glslangPath: ''} -export const onConfigChange = async (change) => { +let supress = false + +export async function onConfigChange(change: DidChangeConfigurationParams) { const temp = change.settings.mcglsl as Config if (temp.shaderpacksPath === conf.shaderpacksPath && temp.glslangPath === conf.glslangPath) return conf = {shaderpacksPath: temp['shaderpacksPath'].replace(/\\/g, '/'), glslangPath: temp['glslangValidatorPath'].replace(/\\/g, '/')} @@ -31,8 +34,14 @@ export const onConfigChange = async (change) => { serverLog.debug(() => 'old config: ' + JSON.stringify(conf)) if (conf.shaderpacksPath === '' || conf.shaderpacksPath.replace(dirname(conf.shaderpacksPath), '') !== '/shaderpacks') { + if (supress) return serverLog.error(() => 'shaderpack path not set or doesn\'t end in \'shaderpacks\'', null) - connection.window.showErrorMessage('mcglsl.shaderpacksPath is not set or doesn\'t end in \'shaderpacks\'. Please set it in your settings.') + supress = true + const clicked = await connection.window.showErrorMessage( + 'mcglsl.shaderpacksPath is not set or doesn\'t end in \'shaderpacks\'. Please set it in your settings.', + {title: 'Supress'} + ) + supress = (clicked && clicked.title === 'Supress') ? true : false return } diff --git a/server/src/graph.ts b/server/src/graph.ts index 83c2b58..b904452 100644 --- a/server/src/graph.ts +++ b/server/src/graph.ts @@ -4,7 +4,7 @@ export type Pair = { second: S } -type Node = { +export type Node = { parents: Map> children: Map } diff --git a/server/src/linter.ts b/server/src/linter.ts index ebddef8..cf55a04 100644 --- a/server/src/linter.ts +++ b/server/src/linter.ts @@ -6,7 +6,7 @@ import { readFileSync, existsSync, statSync, Stats } from 'fs' import { conf } from './config' import { postError, formatURI, getDocumentContents, trimPath } from './utils' import { platform } from 'os' -import { Graph } from './graph' +import { Graph, Node } from './graph' import { Comment } from './comment' import { linterLog } from './logging' @@ -91,7 +91,7 @@ export function preprocess(lines: string[], docURI: string) { const includeMap = new Map(Array.from(allIncludes).map(obj => [obj.path, obj]) as [string, IncludeObj][]) - lint(docURI, lines, includeMap, diagnostics) + lint(docURI, lines, includeMap, diagnostics, hasDirective) } function includeDirective(lines: string[], docURI: string): boolean { @@ -123,11 +123,24 @@ function includeDirective(lines: string[], docURI: string): boolean { const buildIncludeGraph = (inc: IncludeObj) => includeGraph.setParent(inc.path, inc.parent, inc.lineNum) function processIncludes(lines: string[], incStack: string[], allIncludes: Set, diagnostics: Map, hasDirective: boolean) { + // todo figure out why incStack[0] here const includes = getIncludes(incStack[0], lines) includes.forEach(i => allIncludes.add(i)) - if (includes.length > 0) { - linterLog.info(() => `${trimPath(incStack[0])} has ${includes.length} include(s). [${includes.map(i => '\n\t\t' + trimPath(i.path))}\n\t]`) - includes.reverse().forEach(inc => { + + const parent = incStack[incStack.length - 1] + includeGraph.nodes.get(parent).children.forEach((node, uri) => { + if (!includes.has(uri)) { + includeGraph.nodes.get(parent).children.delete(uri) + node.parents.delete(parent) + } + }) + + const includeList = Array.from(includes.values()) + + if (includeList.length > 0) { + linterLog.info(() => `${trimPath(parent)} has ${includeList.length} include(s). [${includeList.map(i => '\n\t\t' + trimPath(i.path))}\n\t]`) + + includeList.reverse().forEach(inc => { buildIncludeGraph(inc) mergeInclude(inc, lines, incStack, diagnostics, hasDirective) }) @@ -137,16 +150,14 @@ function processIncludes(lines: string[], incStack: string[], allIncludes: Set((out, line, i): IncludeObj[] => processLine(out, line, lines, i, lineInfo), []) + return new Map(lines.reduce((out, line, i) => processLine(out, line, lines, i, lineInfo), []).map(inc => [inc.path, inc] as [string, IncludeObj])) } // TODO can surely be reworked @@ -191,7 +202,7 @@ function processLine(includes: IncludeObj[], line: string, lines: string[], i: n return includes } -function ifInvalidFile(inc: IncludeObj, lines: string[], incStack: string[], diagnostics: Map) { +function ifInvalidFile(inc: IncludeObj, lines: string[], incStack: string[], diagnostics: Map, hasDirective: boolean) { const msg = `${trimPath(inc.path)} is missing or an invalid file.` linterLog.error(msg, null) @@ -212,7 +223,7 @@ function ifInvalidFile(inc: IncludeObj, lines: string[], incStack: string[], dia line: inc.lineNum, msg, file, } - propogateDiagnostic(error, diagnostics) + propogateDiagnostic(error, diagnostics, hasDirective) } function mergeInclude(inc: IncludeObj, lines: string[], incStack: string[], diagnostics: Map, hasDirective: boolean) { @@ -221,14 +232,14 @@ function mergeInclude(inc: IncludeObj, lines: string[], incStack: string[], diag stats = statSync(inc.path) } catch (e) { if (e.code === 'ENOENT') { - ifInvalidFile(inc, lines, incStack, diagnostics) + ifInvalidFile(inc, lines, incStack, diagnostics, hasDirective) return } throw e } if (!stats.isFile()) { - ifInvalidFile(inc, lines, incStack, diagnostics) + ifInvalidFile(inc, lines, incStack, diagnostics, hasDirective) return } @@ -248,7 +259,7 @@ function mergeInclude(inc: IncludeObj, lines: string[], incStack: string[], diag lines.splice(inc.lineNumTopLevel + 1 + dataLines.length, 0, `#line ${inc.lineNum + 1} "${inc.parent}"`) } -function lint(docURI: string, lines: string[], includes: Map, diagnostics: Map) { +function lint(docURI: string, lines: string[], includes: Map, diagnostics: Map, hasDirective: boolean) { let out: string = '' try { execSync(`${conf.glslangPath} --stdin -S ${ext.get(path.extname(docURI))}`, {input: lines.join('\n')}) @@ -261,7 +272,7 @@ function lint(docURI: string, lines: string[], includes: Map if (!diagnostics.has(key)) diagnostics.set(key, []) }) - processErrors(out, docURI, diagnostics) + processErrors(out, docURI, diagnostics, hasDirective) daigsArray(diagnostics).forEach(d => { if (win) d.uri = d.uri.replace('file://C:', 'file:///c%3A') @@ -269,7 +280,7 @@ function lint(docURI: string, lines: string[], includes: Map }) } -function processErrors(out: string, docURI: string, diagnostics: Map) { +function processErrors(out: string, docURI: string, diagnostics: Map, hasDirective: boolean) { filterMatches(out).forEach(match => { const error: ErrorMatch = { type: errorType(match[1]), @@ -289,16 +300,16 @@ function processErrors(out: string, docURI: string, diagnostics: Map, parentURI?: string) { +function propogateDiagnostic(error: ErrorMatch, diagnostics: Map, hasDirective: boolean, parentURI?: string) { includeGraph.get(parentURI || error.file).parents.forEach((pair, parURI) => { const diag: Diagnostic = { severity: error.type, - range: calcRange(pair.first - 1, parURI), + range: calcRange(pair.first - (pair.second.parents.size > 0 ? 0 : (2 - (hasDirective ? 1 : 0))), parURI), message: `Line ${error.line} ${trimPath(error.file)} ${replaceWords(error.msg)}`, source: 'mc-glsl' } @@ -307,7 +318,7 @@ function propogateDiagnostic(error: ErrorMatch, diagnostics: Map 0) { - propogateDiagnostic(error, diagnostics, parURI) + propogateDiagnostic(error, diagnostics, hasDirective, parURI) } }) } diff --git a/server/src/server.ts b/server/src/server.ts index cb5ecc7..9fc466b 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -34,7 +34,9 @@ documents.onDidSave((event) => onEvent(event.document)) // what am i saying here // dont do this for include files, for non-include files, clear diags for all its includes. Cache this maybe -documents.onDidClose((event) => connection.sendDiagnostics({uri: event.document.uri, diagnostics: []})) +documents.onDidClose((event) => { + connection.sendDiagnostics({uri: event.document.uri, diagnostics: []}) +}) //documents.onDidChangeContent(onEvent) @@ -48,7 +50,9 @@ export function onEvent(document: vsclangproto.TextDocument) { } // i think we still need to keep this in case we havent found the root of this files include tree - if (!ext.has(extname(document.uri))) return + const lines = document.getText().split('\n') + const hasVersion = lines.filter(l => reVersion.test(l)).length > 0 + if (!ext.has(extname(document.uri)) || !hasVersion) return try { preprocess(document.getText().split('\n'), uri) diff --git a/server/src/utils.ts b/server/src/utils.ts index a7e8766..16a7360 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -5,7 +5,8 @@ import { serverLog } from './logging' export function postError(e: Error) { connection.window.showErrorMessage(e.message) - serverLog.error(e.message, new Error()) + serverLog.error(e.message, null) + console.log(e) } export const formatURI = (uri: string) => uri.replace(/^file:\/\//, '').replace(/^(?:\/)c%3A/, 'C:').replace(/\\/g, '/')