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

Commit 759f7c2

Browse files
committed
fix: error on CSSLint stderr
If CSSLint returns text on `stderr`, or more likely a message is thrown for a miss-configured path to CSSLint, show this message to the user instead of attempting to go on and parse the (empty) output. Messages are shown until the user interacts with them, but are deduplicated so they are only actively shown once. Fixes #181. Fixes #189.
1 parent a24dfa2 commit 759f7c2

File tree

2 files changed

+72
-1
lines changed

2 files changed

+72
-1
lines changed

lib/main.js

+41-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ const loadDeps = () => {
2323
}
2424
};
2525

26+
const getCheckNotificationDetails = checkDetail =>
27+
(notification) => {
28+
const { detail } = notification.getOptions();
29+
if (detail === checkDetail) {
30+
return true;
31+
}
32+
return false;
33+
};
34+
2635
export default {
2736
activate() {
2837
this.idleCallbacks = new Set();
@@ -47,11 +56,15 @@ export default {
4756
'linter-csslint.executablePath',
4857
(value) => { this.executablePath = value; },
4958
));
59+
60+
this.activeNotifications = new Set();
5061
},
5162

5263
deactivate() {
5364
this.idleCallbacks.forEach(callbackID => window.cancelIdleCallback(callbackID));
5465
this.idleCallbacks.clear();
66+
this.activeNotifications.forEach(notification => notification.dismiss());
67+
this.activeNotifications.clear();
5568
this.subscriptions.dispose();
5669
},
5770

@@ -90,7 +103,34 @@ export default {
90103

91104
const execPath = this.determineExecPath(this.executablePath, projectPath);
92105

93-
const output = await helpers.exec(execPath, parameters, execOptions);
106+
let output;
107+
try {
108+
output = await helpers.exec(execPath, parameters, execOptions);
109+
} catch (e) {
110+
// eslint-disable-next-line no-console
111+
console.error(e);
112+
113+
// Check if the notification is currently showing to the user
114+
const checkNotificationDetails = getCheckNotificationDetails(e.message);
115+
if (Array.from(this.activeNotifications).some(checkNotificationDetails)) {
116+
// This message is still showing to the user!
117+
return null;
118+
}
119+
120+
// Notify the user
121+
const message = 'linter-csslint:: Error while running CSSLint!';
122+
const options = {
123+
detail: e.message,
124+
dismissable: true,
125+
};
126+
const notification = atom.notifications.addError(message, options);
127+
this.activeNotifications.add(notification);
128+
// Remove it when the user closes the notification
129+
notification.onDidDismiss(() => this.activeNotifications.delete(notification));
130+
131+
// Don't update any current results
132+
return null;
133+
}
94134

95135
if (textEditor.getText() !== text) {
96136
// The editor contents have changed, tell Linter not to update

spec/linter-csslint-spec.js

+31
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,35 @@ describe('The CSSLint provider for Linter', () => {
114114
expect(foundPath).toBe('foobar');
115115
});
116116
});
117+
118+
describe('handles invalid CSSLint paths', () => {
119+
let editor;
120+
const message = 'linter-csslint:: Error while running CSSLint!';
121+
const detail = 'Failed to spawn command `foo`. Make sure `foo` is installed and on your PATH';
122+
123+
beforeEach(async () => {
124+
atom.config.set('linter-csslint.executablePath', 'foo');
125+
126+
editor = await atom.workspace.open(goodPath);
127+
await lint(editor);
128+
});
129+
130+
it('tells the user when they specify an invalid CSSLint path', async () => {
131+
const currentNotifications = atom.notifications.getNotifications();
132+
133+
expect(currentNotifications.length).toBe(1);
134+
expect(currentNotifications[0].getMessage()).toBe(message);
135+
expect(currentNotifications[0].getOptions().detail).toBe(detail);
136+
});
137+
138+
it('only notifies for invalid paths once', async () => {
139+
// Run the lint again to check the path twice
140+
await lint(editor);
141+
const currentNotifications = atom.notifications.getNotifications();
142+
143+
expect(currentNotifications.length).toBe(1);
144+
expect(currentNotifications[0].getMessage()).toBe(message);
145+
expect(currentNotifications[0].getOptions().detail).toBe(detail);
146+
});
147+
});
117148
});

0 commit comments

Comments
 (0)