Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Optionally omit globals warning #10

Merged
merged 7 commits into from
Jan 12, 2017
Merged

Optionally omit globals warning #10

merged 7 commits into from
Jan 12, 2017

Conversation

nbredikhin
Copy link
Contributor

This is a simple config variable that makes linter ignore "accessing global" warnings. It would be very useful since moonc -l has no support for this.

@@ -58,6 +59,7 @@ export default {

const messages = [];
let match = parseRegex.exec(result);
let globalsOmitted = atom.config.get('linter-moonscript.omitGlobalCheck');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please observe this setting like this, there's no need to go through the complexity of determining what it is and what the current value is on every call 😉.

text: match[2],
range: helpers.rangeFromLineNumber(textEditor, line),
});
if (!globalRegex.test(match[2]) && !globalsOmitted) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ||.

As it currently stands, if match[2] contains accessing global it will always be ignored. If it doesn't contain accessing global then the globalsOmitted option being false lets it show up, but if somebody enabled it then all messages will be hidden.

"omitGlobalCheck": {
"type": "boolean",
"default": false,
"description": "Omit \"accessing global\" warning."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"warnings"

@Arcanemagus
Copy link
Member

Sorry it took so long to get this reviewed!

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Arcanemagus Arcanemagus merged commit d742eb3 into AtomLinter:master Jan 12, 2017
@Arcanemagus
Copy link
Member

Published in v1.1.0 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants