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

setname and Standard Replies #735

Merged
merged 3 commits into from
Jan 30, 2025
Merged

setname and Standard Replies #735

merged 3 commits into from
Jan 30, 2025

Conversation

andymandias
Copy link
Collaborator

Simple, straightforward implementations for Standard Replies and setname. Bundled because setname will produce a Standard Reply when it fails, providing an easy way to trigger a FAIL Standard Reply (on InspIRCd servers).

Standard Replies introduces three new colors to themes (for FAIL, WARN, and NOTE), which default to the server message color if not specified. Standard replies all all shown in the server buffer (triggering the unread indication).

Copy link
Member

@casperstorm casperstorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean. Haven't tested it but code looks good 👍🏻

@4e554c4c
Copy link
Contributor

oh awesome! I was thinking about taking a shot at this, so im glad it's getting done. This would also resolve #656.

One quick comment:

Standard Replies introduces three new colors to themes (for FAIL, WARN, and NOTE), which default to the server message color if not specified.

in my opinion FAIL should default to an error color, and WARN to secondary. Both of these are important to not miss, and possibly fatal to the connection. Only NOTE is really an ordinary server message.

@andymandias
Copy link
Collaborator Author

Error color makes sense to me for FAIL. I'm not sure if text-secondary will attract attention, at least with the default Ferra theme, but that might just be me. Any preference for the fallback WARN coloring, @casperstorm?

… color.

Log capabilities that are deleted by the server.
Copy link
Member

@tarkah tarkah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean! LGTM

@andymandias andymandias merged commit d7ecb5f into main Jan 30, 2025
1 check passed
@andymandias andymandias deleted the feat/setname branch January 30, 2025 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants