Skip to content

Commit

Permalink
Merge branch 'linting'
Browse files Browse the repository at this point in the history
  • Loading branch information
awmcclain committed May 18, 2018
2 parents 18adab6 + b82b479 commit 80d18e4
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 54 deletions.
75 changes: 35 additions & 40 deletions app/linter/ink-linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ var defaultDir = '../Assets/Ink Files/';
// Run just against GDC rides
var filesToLint = defaultDir + 'GDCDemo/Rides/*.ink';


var sendOutputToStream = false;
var collectedOutput;
// OR, run against all Rides
// var filesToLint = defaultDir + '{,GDCDemo/}' + "Rides/{**,!(NotInUse)/}*.ink"

Expand All @@ -18,9 +15,9 @@ var collectedOutput;
/* TODO:
Implement:
1. react tag with an emote arg, and vice versa
invalid: >>react:happy >>emote:squint
2. Add test and fix: When pause has a parameter with a dot (>>pause:1.2), the linter believes this is an error.
1. react tag with an emote arg, and vice versa - invalid: >>react:happy >>emote:squint
2. discuss the new, skipped test with Andrew. Is it worth trying to handle ink tags with colons, or are we just
reimplementing the Inky tag-parser.
3. (Defer) Character tags with mis-spelled characters
/* REGEX PATTERNS */
Expand Down Expand Up @@ -138,7 +135,7 @@ function hasCharacterTagError(matchObject) {
let needsParams = tagsAndLinting[matchObject.tagName] && tagsAndLinting[matchObject.tagName].needsParam === true;

if (needsParams && !decoratedMatchObject.parameter) {
logBadTag(`Character tag ${matchObject.tagName} requires params, but none found`, matchObject);
logBadTag(`Character tag requires params, but none found`, matchObject);
return true;
}

Expand All @@ -148,17 +145,34 @@ function hasCharacterTagError(matchObject) {
return false;
}

// Linting to confirm character tags have a name and valid type
// Linting to confirm character tags have a name and valid type

function checkForInValidCharacterTagForm(decoratedMatchObject) {
if (!decoratedMatchObject.character || !decoratedMatchObject.type) {
logBadTag(`Character tag missing or malformed: ${decoratedMatchObject.fullTag}`, decoratedMatchObject);
if (!decoratedMatchObject.character) {
logBadTag('Could not extract/identify character name associated with character tag', decoratedMatchObject);
return true;
}
}

if(!decoratedMatchObject.type) {
logBadTag('Character type (inline or leadingName) could not be determined', decoratedMatchObject);
return true;
}

return false;
}


function isValidPauseTag(matchObject) {
let isInteger = string => typeof(parseInt(string)) === 'number';

if (matchObject.tagName === 'pause' && isInteger(matchObject.semiColonArg) && (!matchObject.periodArg || isInteger(matchObject.periodArg))) {
return true;
}

return false;
}


// Function that identifies the character name and parameters
// for both of the two distint character types (inline and leading)
function decorateCharTags(matchObject) {
Expand Down Expand Up @@ -197,7 +211,7 @@ function decorateCharTags(matchObject) {
function hasStoryTagError(matchObject) {
// story tags must have an argument after the semiColon
if (!matchObject.semiColonArg) {
logBadTag(`Story tag '${matchObject.tagName}' missing argument`, matchObject);
logBadTag('Story tag missing argument', matchObject);
return true;
}

Expand All @@ -206,8 +220,10 @@ function hasStoryTagError(matchObject) {


//story tags must have no argument after the period
if (matchObject.periodArg) {
logBadTag(`Story tag '${matchObject.tagName}' malformed: ${matchObject.fullTag}`, matchObject);
//unless it's a valid decimal-containing pause story tag
if (matchObject.periodArg && !isValidPauseTag(matchObject)) {

logBadTag('Story tag malformed', matchObject);
return true;
}

Expand All @@ -217,7 +233,7 @@ function hasStoryTagError(matchObject) {
function hasUITagError(matchObject) {
// story tags must have an argument after the semiColon
if (!matchObject.semiColonArg) {
logBadTag(`UI tag '${matchObject.tagName}' missing argument`, matchObject);
logBadTag('UI tag missing argument', matchObject);
return true;
}

Expand All @@ -227,7 +243,7 @@ function hasUITagError(matchObject) {
function hasInvalidParam(matchObject, param) {
let validParams = tagsAndLinting[matchObject.tagName] && tagsAndLinting[matchObject.tagName].validParams;
if (validParams && !validParams.includes(param.toLowerCase())) {
logBadTag(`Parameter for '${matchObject.tagName}' is not allowed: ${matchObject.fullTag}`, matchObject);
logBadTag('Parameter for that tag is not allowed', matchObject);
return true;
}
return false;
Expand All @@ -242,36 +258,16 @@ function isCommented(line) {

function logBadTag(message, matchObject) {
let errorDetails = `${message}\n` + colors.yellow(matchObject.line);
if (sendOutputToStream) {
collectedOutput += `WARNING: '${matchObject.path}' line ${matchObject.lineIndex}: ${message}\n`;
} else {
console.log(`WARNING: '${matchObject.path}' line ${matchObject.lineIndex}: ${errorDetails}`);
}
console.log(`WARNING: '${matchObject.path}' line ${matchObject.lineIndex}: ${errorDetails}`);
}

function lintInkFile(path) {
let text = fs.readFile(path, 'utf8', function(err, data) {
lintBuffer(data);
});
}

function lintBuffer(path, data) {
let lines = data.split("\n");
for (let i = 0; i < lines.length; i++) {
hasLineErrors(lines[i], i + 1, path);
}
}

function lintFilesFromInky(inkFileBuffers) {
sendOutputToStream = true;
collectedOutput = "";

for (var path in inkFileBuffers) {
console.log(`Linting ${path}`);
lintBuffer(path, inkFileBuffers[path]);
}

return collectedOutput;
});
}

function hasLineErrors(line, lineNumber, path) {
Expand Down Expand Up @@ -301,7 +297,7 @@ function hasLineErrors(line, lineNumber, path) {
if (lintingErrorFunction(matchObject)) lineError = true;

} else {
logBadTag(`Unknown tag: '${matchObject.tagName}'`, matchObject);
logBadTag('Unknown tag name', matchObject);
lineError = true;
}
}
Expand All @@ -321,4 +317,3 @@ glob(filesToLint, function( err, files ) {
});

module.exports = hasLineErrors;
module.exports.lintFilesFromInky = lintFilesFromInky;
24 changes: 23 additions & 1 deletion app/linter/linter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,18 @@ test('handle Ink tags prior to the name on line-first', t => {
}
})


test.skip('Ignore ink tags prior to the name when ink tags have colons', t => {
let tags = [
' {isRed(): RISA: Well the important thing is to ensure the safety of all, right? >>react:awkward }',
'Cute:RISA: okay >>emote:sad'
]

for (let tag of tags) {
t.false(hasLineErrors(tag, 0, 'test.js'))
}
})

test('Test valid story tags with parameters', t => {
let tags = [
'>>cutCamera:paxLeft',
Expand All @@ -108,6 +120,16 @@ test('Test valid story tags with parameters', t => {
}
});

test('Do not throw errors for pause story tags with decimal', t => {
let tags = [
'>>pause:1.5',
'>>pause:0.7']

for (let tag of tags) {
t.false(hasLineErrors(tag, 0, 'test.js'))
}
});


test('Test story tags missing required parameters', t => {
t.true(hasLineErrors('>>cutCamera', 0, 'test.js'));
Expand All @@ -125,4 +147,4 @@ test('Test story tags missing required parameters', t => {

test('valid UI tags should be accepted', t => {
t.false(hasLineErrors('>>UIView:MapView.RideDetails'));
})
})
20 changes: 10 additions & 10 deletions app/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
"url": "https://github.com/awmcclain/slinky/issues"
},
"devDependencies": {
"ava": "^0.25.0",
"colors": "^1.2.1",
"electron": "^1.4.3",
"markdown-html": "^0.0.8",
"spectron": "^3.2.6",
"ava": "^0.25.0",
"colors": "^1.2.1"
"spectron": "^3.2.6"
},
"dependencies": {
"chokidar": "^1.6.0",
Expand Down

0 comments on commit 80d18e4

Please sign in to comment.