use debug instead of console.warn (https://www.npmjs.com/package/debug)

[briangonzalez]

So I use osa-imessage for alfred-messages. These console.warn's break Alfred functionality because Alfred tries to parse stdout and osa-imessage is littering it.

debug is super popular and great for this sort of use case.

Thoughts?

[wtfaremyinitials]

I see the necessity for a way to suppress debug style prints, but I'm not entirely sure that silencing them by default is the right way to go (as the debug module does). It's super important that someone using osa-imessage on an untested version gets that message because it could break in weird and potentially damaging ways (corrupting data, getting in a loop sending out infinite messages, etc).

For your use-case specifically, does Alfred parse standard error (fd=2) in addition to standard out (fd=1)? Printing to stderr over stdout might be a better default.

[briangonzalez]

Fair.

Although converting the warns to errors still seems to result in the issue:

Note: This is from the Alfred debugger output.

[wtfaremyinitials]

Interesting. Can you pass environment variables from Alfred?

[briangonzalez]

Yessir. Thinking about a verbose flag which defaults to on?

[wtfaremyinitials]

How about a SILENCE_OSA_IMESSAGE environment variable which defaults to false?

[briangonzalez]

Unfortunately it doesn't look like it.

[wtfaremyinitials]

Doesn't look like you can pass environment variables from Alfred?

[briangonzalez]

Correct. I use alfy and it, in turn, uses run-node.

[wtfaremyinitials]

How about this?

A property SUPPRESS_WARNINGS on module.exports that defaults to false. Check that it's still false before each debug print.

To make this remove the untested version print that check will have to be moved to a setImmediate call.

[briangonzalez]

I'll give this a shot. Standby.

[briangonzalez]

@wtfaremyinitials Here's another thought – what if the API for the library changed slightly?

const imessage = require('osa-imessage')()

This would allow the end developer to configure the library on a per-need basis.

const imessage = require('osa-imessage')({
  silent: true
})

[wtfaremyinitials]

I suppose that does work but I'd really rather not make a breaking API change at the top level for a feature that will be used so rarely

[briangonzalez]

Ok, then do you have examples of the implementation above that you're referencing? Not sure I follow.

[wtfaremyinitials]

I went ahead and implemented it on master. How's this?

https://github.com/wtfaremyinitials/osa-imessage/commit/427b9f59e638665581a171d8f987188a1485f83a

[wtfaremyinitials]

If that implementation works for your use case I'll go ahead and publish a new version on npm.

[briangonzalez]

Let me give it a shot.

[briangonzalez]

Hmmm, no dice.

const imessage = require('./')
imessage.SUPPRESS_WARNINGS = true

Output:

▵  node ./test.js
This version of macOS (10.12.6) is currently untested with this version of osa-imessage. Proceed with caution.

[briangonzalez]

This works, however:

// test.js
process.env.SUPPRESS_OSA_IMESSAGE_WARNINGS = true
const imessage = require('./')

function warn(str) {
+    if (!process.env.SUPPRESS_OSA_IMESSAGE_WARNINGS) {
        console.error(ol(str))
    }
}

[briangonzalez]

Well, hmm, no wonder the first doesn't work. I am setting the value post require.

[wtfaremyinitials]

Ohh I forgot to do the setImmediate bit in my implementation. Although I like your solution of setting the env variable within node better. I’ll revert my commit & merge yours.

[briangonzalez]

Cool. Here ya go.

https://github.com/wtfaremyinitials/osa-imessage/pull/16