Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore proper pause tags #3

Merged
merged 1 commit into from
May 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Ignore proper pause tags
  • Loading branch information
gingeranyhow committed May 1, 2018
commit 96950272e0caa463ef9f0e6232aa557bd2dc499a
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