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

Check invalid fields before passing off to PowerShell #7

Open
shful opened this issue Feb 14, 2017 · 6 comments
Open

Check invalid fields before passing off to PowerShell #7

shful opened this issue Feb 14, 2017 · 6 comments
Labels

Comments

@shful
Copy link

shful commented Feb 14, 2017

The main example code

func main() {
    notification := toast.Notification{
        AppID: "Example App",
        Title: "My notification",
        Message: "Some message about how important something is...",
        Icon: "go.png",
        Actions: []toast.Action{
            {"protocol", "I'm a button", ""},
            {"protocol", "Me too!", ""},
        },
    }
    err := notification.Push()
    if err != nil {
        log.Fatalln(err)
    }
}

works very different from the example screenshots. Tested with both Win10x64 and Win10x32.
Only the "Example App" is shown, and a kind of "you have a new notification" message.

Title, Message and Buttons are missing.

bildschirmfoto 2017-02-14 um 14 26 38

@jmshal
Copy link
Member

jmshal commented Feb 14, 2017

Hi @shful, thanks for filing this issue. Would you be able to give me just a little more information;

  • Are you using the CLI, or programmatically?
    • If programmatically, does the CLI work for you as intended? Or is that not working either?
  • Which branch/pkg are you importing into your project? (master or gopkg)
  • What version of Windows 10 are you running?

Also, could you provide the code you're trying to run, which isn't working correctly? I've run into issues in the past where some UTF-16 characters I've used cause the buttons to not show up.

@shful
Copy link
Author

shful commented Feb 15, 2017

Hi Jacob, the example run programmatically; it is exactly the code from here: https://github.com/go-toast/toast#example . Toast was obtained with go get gopkg.in/toast.v1

To compare, I cloned the current master (Commit a15160) and it made no difference.
My Windows installation is Windows 10.0.14393 Pro (i.e. with latest updates)

Concerning the encoding: I see that in %TEMP%, a .ps1 file appears which is UTF-8 with Unix line endings.
I don't know which encoding Microsoft .ps1 files require. I tested with non-ASCII ("ä", which is two bytes in UTF-8) in the appId. This is displayed wrong (as two chars) by windows.

@jmshal
Copy link
Member

jmshal commented Feb 15, 2017

Thank you @shful, I will get back to you shortly.

Edit: You picked a perfect week to file this issue, as my Mac is in for repair, and I'm having to use my PC at the moment. Haha.

@jmshal
Copy link
Member

jmshal commented Feb 15, 2017

I have been able to replicate the issue. Crazy that it has all of a sudden stopped working - I'm thinking there must have been a breaking change to the API (which I find unlikely). I am looking further into the issue. I appreciate you making me aware of this, seeing as no changes have been made to this codebase in some time.

@jmshal
Copy link
Member

jmshal commented Feb 15, 2017

Sorry @shful, it's been awhile since I've been in this repo. I thought it was an issue in the library, however the example is not really that useful, as it doesn't explain that the "go.png" image needs to be available on disk. It seems like Microsoft decided that this API was not going to throw a hissy fit when arguments are passed to it which are invalid.

In order to mitigate this from ever happening again, I will file another issue to ensure that the library does some checks (which should really be done in the notification API, to be honest), and throw an error prematurely, to prevent being running into this issue themselves.

Because this will be a breaking change, I may include this in a future release. I would not like to break people's apps due to this addition.

Thanks again, @shful, I hope this all makes sense.

Edit: For the meantime, I will update the example so hopefully people don't run into the same issue.

@jmshal jmshal added feature and removed bug labels Feb 15, 2017
@jmshal jmshal closed this as completed Feb 15, 2017
@jmshal jmshal reopened this Feb 15, 2017
@jmshal jmshal changed the title Example code shows the "AppID" text only Check invalid fields before passing off to PowerShell Feb 15, 2017
@shful
Copy link
Author

shful commented Feb 15, 2017

Hello Jacob,
Indeed, it is sufficient to not initialize the Icon:

func main() {
    notification := toast.Notification{
        AppID: "Example App",
        Title: "My notification",
        Message: "Some message about how important something is...",
        Actions: []toast.Action{
            {"protocol", "I'm a button", ""},
            {"protocol", "Me too!", ""},
        },
    ...

and it works.
Initially, I indeed considered to test the image thing, but that seemed a very unlikely reason ;-)

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

No branches or pull requests

2 participants