Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

In watch mode, play an audible cue when there is an error. #1578

Merged
merged 3 commits into from
Jun 8, 2016

Conversation

PeterHatch
Copy link
Contributor

This adds a feature I like from the CoffeeScript compiler - when in watch mode, make an audible noise if there is an error processing a watched file.

@xzyfer
Copy link
Contributor

xzyfer commented Jun 6, 2016

Thanks @PeterHatch. I'm not opposed to this but it certainly should not be the default. If we were adopt something like this it would have be opt-in.

@saper
Copy link
Member

saper commented Jun 6, 2016

Given the amount of noise characters generated by various tools those days (ANSI colors for example) this seems harmless in comparison. I am relying a lot on screen + xterm + X window system to process a bell beep as the X11 urgent notification. Can we check if we talk to the terminal before we generate a BEL? Is Windows working fine here?

@PeterHatch
Copy link
Contributor Author

I updated it to be an option (--error-bell) - definitely open to changing the name and/or description, if desired.

Windows is working fine, and is what I use (Windows 8.1, anyway).

For reference, CoffeeScript does this automatically when watching, and there was an issue about making it optional that was rejected - jashkenas/coffeescript#3346.

Also an issue for adding it as an option to Webpack - webpack/webpack#388 - which was rejected in favor of making a plugin for that behavior.

@xzyfer
Copy link
Contributor

xzyfer commented Jun 7, 2016

My hesitation for this to be the default is because we get lots of issues over every trivial change. Some of the issues raise on the coffeescript issue are real, I know I'd hate to start receiving bells.

Could you also please update the usage documentation in the readme - https://github.com/sass/node-sass#usage-1

@PeterHatch
Copy link
Contributor Author

Updated the usage documentation.

And yeah, reading the coffeescript issue convinced me it should be an option. I also like having any documentation of it, as it's most useful once you know to expect it. And making it an option means you can turn it on when not using watch mode, which I think may be useful when doing things like using another program (like nodemon) to do the watching.

@xzyfer
Copy link
Contributor

xzyfer commented Jun 8, 2016

Thanks for your contribution @PeterHatch!

@xzyfer xzyfer added this to the next.minor milestone Jun 8, 2016
@xzyfer xzyfer merged commit f912216 into sass:master Jun 8, 2016
@PeterHatch PeterHatch deleted the watch-error-audio-cue branch June 12, 2016 02:17
@xzyfer xzyfer modified the milestone: next.minor Sep 4, 2016
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.

3 participants