Add login retry functionality

[geisler]

This patch adds a new setting in the config file: num_login_tries. The default value is 1 to match the current functionality. If the value is set larger, the program will retry up to this many times until there is a successful login, or the number of tries has been exhausted.

In order to correctly accomplish this new piece of functionality, I had to add two values to the settings struct to keep track of the values of the username and password from the config file separate from those that have been created by looking them up so that they can be reset upon failure.

Lightly tested without any problems.

[PromyLOPh]

What is the problem you are trying to solve with this change?

Note that currently there is no retry logic in place for any user-initiated action and this change would create an exception to that rule.

[geisler]

Typing in the wrong password is the most common issue. It just feels better to be re-prompted for a new username & password like every other UI than to just exit the program and have to restart it. All other user actions continue running the program, so this is an exceptional situation.

[PromyLOPh]

All other user actions continue running the program, so this is an exceptional situation. The situation is exceptional, because pianobar simply cannot continue without successful authentication. I’m fine with restarting the program in this case.

Also the proposed change fails with a double-free error whenever an invalid user/password is used.

[geisler]

I believe I've fixed the double free for what that is worth.

Obviously, I believe the extra request is worth it, but it is a subjective taste and as the owner of the project you have the final say.

[PromyLOPh]

It’s not just taste, imo. There’s open questions, like:

• Why do I have to retype user and password if it’s just a (transient) network issue?
• Why do I have to retype the username if only the password is wrong?
• Why is it retrying if user or password in the config file are wrong? And so on…

Btw: I don’t consider myself the “owner” of this project.

[geisler]

Good questions. Here are my attempts at answers:

• The username and password must be retyped for a (transient) network issue with the current implementation, so this solution is no better or worse.
• We need to retype to username because Pandora does not give us an indication which entry is the issue. Again, same as the current implementation.
• With the username and password in the configuration, presumably the user would not set the retry option above 1. IMO, it only makes sense to tell pianobar to retry when the configuration does not contain a username or password. As an aside, I like having my username set in the config file, but don't have my password there for security reasons. This patch is really nice because as a user I only have to re-enter the password because I know I have the right username in the file and it gets automatically populated for me. It is very convenient.

In my mind, the only user activity that changes with the patch is not having to restart the program. All other user behavior is the same as the current implementation. On top of that, the default behavior is exactly the same as the current implementation so that only those who ask for this different behavior receive it. The program always behaves the same until it is asked to do something different.

[PromyLOPh]

So if we’re not going to activate it for everyone why not use a shell script like

for i in `seq 1 3`; do
    pianobar || exit 1
done

? (I’m aware there’s currently no meaningful exit codes, but that’s a trivial change.)

[geisler]

That basically does the same thing, but seems strange to implement in the following sense: The patch I'm submitting allows the user to configure the number of retries in ~/.config/pianobar/config along with all the other configuration variables from the application. This script would be a second location to configure the program and feels disjointed from the rest of the application and unlikely to be used.

[PromyLOPh]

Sure, it’s not as “user-friendly” (whatever that might be) as a built-in mechanism. But it does exactly the same thing like your patch without adding any code to pianobar. Since I’d be the one maintaining this feature I’m obviously happy with an external shell script.