-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add password_command for sasl and nick_password
#583
Conversation
This commit builds off of squidowl#552 and allows the user to supply a command instead of a password file anywhere that a password file is used. If the password command ends unsuccessfully, then an error is displayed. Trailing whitespace is now trimmed from password commands, as it is commonplace for commands to leave a trailing newline after the password they output. Resolves squidowl#580
password_command for sasl and
nick_password`nick_password
I think this deserves a entry in CHANGELOG. I'll do proper review tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A single nit regarding naming, but overall it looks great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. @tarkah do you have anything for this?
f6fe926
to
2e8a375
Compare
#[error("Exactly one of sasl.plain.password, sasl.plain.password_file or sasl.plain.password_command must be set.")] | ||
DuplicateSaslPassword, | ||
#[error("Config does not exist")] | ||
ConfigMissing { has_yaml_config: bool }, | ||
} | ||
|
||
impl From<std::io::Error> for Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to refactor this bit out (the custom From
impl) for this PR, but it turns out that every single Error
in Halloy has to be Clone
im not a huge fan but also fixing that is out of scope for this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look great! I think that cleaned it up very nicely. Thanks for tackling this
Thanks for yet another good PR. |
This commit builds off of #552 and allows the user to supply a command instead of a password file anywhere that a password file is used. If the password command ends unsuccessfully, then an error is displayed.
Trailing whitespace is now trimmed from password commands, as it is commonplace for commands to leave a trailing newline after the password they output.
Resolves #580