gotify / cli

A command line interface for pushing messages to gotify/server.
MIT License
444 stars 58 forks source link

fix panic when no config and no priority argument #61

Closed tessus closed 1 year ago

tessus commented 1 year ago

fixes #50

jmattheis commented 1 year ago

Thanks

tessus commented 1 year ago

In issue #50 it was stated

But the priority has been claimed that it has a default value with 0.

But after your change it leaves priority undefined. There is no else branch. I checked the rest of the code, but priority is nowhere to be set to 0.

jmattheis commented 1 year ago

There is no "undefined" in Go, priority is an int meaning it cannot be undefined/nil it always has a value. It this case 0 is returned by the cli library, when a value is unset.

tessus commented 1 year ago

Well, if I test the code like so:

The message is pushed to the clients, but there is no notification. If I add -p X, there is a notification. There are no errors in the server logs. I really think you should have left the else branch.

But maybe there is no notifiction for prio 0 messages, but I did that in my tests for the PR and I got a notification for prio 0 messages.

tessus commented 1 year ago

@jmattheis

Since I wasn't able to find any documentation on this...

What is the highest priority and the lowest priority? Is 10 the highest or the lowest? Also, the priority is not shown in the client (web ui or Android), so why is there a priority? Unfortunately the docs do not explain priorities at all.

Can you please shed some light on this?

jmattheis commented 1 year ago

There are no errors in the server logs. I really think you should have left the else branch.

You probably had a default priority configured. This higher priority then caused the notification.


The webui colors the left border depending on the priority. gotify/android priorities are defined here https://github.com/gotify/android#message-priorities

Highest priority is int max and lowest int min.

tessus commented 1 year ago

Awesome, thanks. Any chance this info can be referenced in the docs?

jmattheis commented 1 year ago

Yeah, it could probably included in the pushmessage page.