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

Multi-line notifications on windows 10 show \n instead of a line break #123

Closed
aplex opened this issue Jun 24, 2016 · 12 comments
Closed

Multi-line notifications on windows 10 show \n instead of a line break #123

aplex opened this issue Jun 24, 2016 · 12 comments

Comments

@aplex
Copy link

aplex commented Jun 24, 2016

Multi-line notifications on windows 10 show "\n" instead of a line break.

Show notification with message line1\nline2

Expected result:

line1
line2

Actual result:
line1\nline2

v4.6.0

Previous version 4.5 showed multi-line notifications on some windows 10 computers, and showed "\n" on others.

@Knagis
Copy link

Knagis commented Jul 1, 2016

I had the exact same issue right now

  • node-notifier 4.6.0
  • Windows 10 build 10586.456
  • node version 6.0.0

@Titozzz
Copy link

Titozzz commented Sep 5, 2016

Same issue here

@mikaelbr
Copy link
Owner

mikaelbr commented Sep 5, 2016

Is this also an issue using this branch? #134

@ew72ew
Copy link

ew72ew commented Dec 31, 2016

The problem is here:
https://github.com/mikaelbr/node-notifier/blob/master/lib/utils.js#L263

Avoiding this string replace will enable line breaks on Windows 10 with toast.exe.

mikaelbr added a commit that referenced this issue Jan 19, 2017
@mikaelbr
Copy link
Owner

This should hopefully be fixed in the latest version in the master branch. Would love it if you could test it out:

npm i mikaelbr/node-notifier

@ew72ew
Copy link

ew72ew commented Jan 19, 2017

It seems to use SnoreToast.exe instead of toast.exe and basically it works now 👍

Unfortunately I am using this through gulp-notify and when it is about to toast an error this happens:

gulp-notify: [Error running notifier] Could not send message: Command failed: xyz\node_modules\node-notifier\vendor\snoreToast\SnoreToast.exe -open -subtitle -p xyz\node_modules\gulp-notify\assets\gulp-error.png -m message1\r\nmessage2 -t title

In that case it will also send "open" and "subtitle" options which SnoreToast cannot process and fails.

I tried a quick hack in toaster.js which fixed it:

delete options.open;
delete options.subtitle;

Hope it helps.

Edit: The default sound behavior has also changed. In the previous version, only the "error" toast produced this default sound while the "success" toasts were quiet.

Edit2: Wait, it did not work. I forgot that I temporary removed the removeNewLines() call. Maybe this helps: ew72ew@2751578

@mikaelbr
Copy link
Owner

Thanks! It seems SnoreToast doesn't allow surplus arguments, so need to filter out things and not let notifier be transient with options as with notification center on Mac. Thanks for the code & comments! I've got access to windows versions at work now, so I can test and fix it within the day.

@mikaelbr
Copy link
Owner

@ew72ew How is the master branch now? I've added several tests, implemented better support for all-things snoretoasts.

@ew72ew
Copy link

ew72ew commented Jan 25, 2017

The linebreaks work now. Definitely!

Now there is only the problem with the default sound left. Since it is gulp-notify related, I am not sure if it should be fixed here or there.

gulp-notify sets this defaults:

var defaults = {
  error: {
    icon: path.join(__dirname, '..', 'assets', 'gulp-error.png'),
    sound: 'Frog'
  },
  regular: {
    icon: path.join(__dirname, '..', 'assets', 'gulp.png')
  }
};

Error message

So if an error happens it will execute the following command:

xyz\SnoreToast.exe -p xyz\gulp-error.png -m Error message -t Title -s Frog

However SnoreToast.exe fails if "Frog" is supplied as the sound. It still displays the toast, plays the default Windows sound but it ignores the custom image, always has the title "SnoreToast" and no message.

To make it work, one of these must be used instead:

# If no sound is supplied, it will always use the default Windows sound
xyz\SnoreToast.exe -p xyz\gulp-error.png -m Error message -t Title

# Alternatively a valid sound must be used
xyz\SnoreToast.exe -p xyz\gulp-error.png -m Error message -t Title -s Notification.Default

A quick hack could be:

  if (options.sound) {
    options.s = options.sound;
    delete options.sound;

    // "Frog" is set by gulp-notify for errors but is not a valid Windows sound
    if (options.s === 'Frog') {
      options.s = 'Notification.Default';
    }
  }

Success message

The success message on the other hand uses the following command:

xyz\SnoreToast.exe -p xyz\gulp.png -m Success message -t Title

And this also generates the default Windows sound. It should be really using this command:

xyz\SnoreToast.exe -p xyz\gulp.png -m Success message -t Title -silent

This is how toast.exe worked. Success messages were silent, so we need the -silent option to prevent the default sound.

Possible solution

  1. Silent should be true by default
  2. If a sound is provided, silent should be removed
  3. Either the "Frog" exception has to be added or another validation to make sure the sound is valid

Hope it helps and thank you for working on it!

@mikaelbr
Copy link
Owner

Thanks for the detailed feedback. The problem is managing sound in a cross platform manner, where if you use Frog on Mac you shouldn't crash on windows. I think maybe one way to solve this is to validate input sound and if it doesn't match use a default sound. I think this is OK as when you choose a sound, it's expected to get a sound, not silent.

@mikaelbr
Copy link
Owner

It should work better now with sound. Still not cross platform in the sense that it is the same sound, but at least it won't crash.

@mikaelbr
Copy link
Owner

Closing this as fix is verified. Will be released in v5 soon.

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

No branches or pull requests

6 participants