command.go: modifying Theme command to show all available themes#232
command.go: modifying Theme command to show all available themes#232shazow merged 1 commit intoshazow:masterfrom ste5e99:theme-stuff
Conversation
chat/command.go
Outdated
| theme = cfg.Theme.ID() | ||
| } | ||
| body := fmt.Sprintf("Current theme: %s", theme) | ||
| avail := "Themes available: " |
There was a problem hiding this comment.
IMO append to a buffer.
var avail bytes.Buffer
fmt.Fprintf(&avail, "Themes available: ")
...
room.Send(message.NewSystemMsg(avail.String(), user))There was a problem hiding this comment.
I amended it but had to add the bytes import as well for bytes.Buffer.
|
LGTM. Wanna figure out why travis is upset? |
|
I filed an issue for that in #231. Edit: im dumb |
|
It's not the same error as #231. This looks like a linting thing. |
|
The linter says Line 125 on - if h.isOp(term.Conn) {
- h.Room.Ops.Add(set.Itemize(member.ID(), member))
+ if key := term.Conn.PublicKey(); key != nil {
+ authItem, err := h.auth.ops.Get(newAuthKey(key))
+ if err == nil {
+ err = h.Room.Ops.Add(set.Rename(authItem, member.ID()))
+ }
+ }
+ if err != nil {
+ logger.Warningf("[%s] Failed to op: %s", term.Conn.RemoteAddr(), err)
}
- ratelimit := rateio.NewSimpleLimiter(3, time.Second*3)
+ ratelimit := rateio.NewSimpleLimiter(3, time.Second*3)
logger.Debugf("[%s] Joined: %s", term.Conn.RemoteAddr(), user.Name())It claims that Edit: changing target branch to master as per previous discussion to develop features from master. |
chat/command.go
Outdated
| @@ -191,6 +192,15 @@ func InitCommands(c *Commands) { | |||
| theme = cfg.Theme.ID() | |||
| } | |||
| body := fmt.Sprintf("Current theme: %s", theme) | |||
There was a problem hiding this comment.
Wanna put the body into the buffer too, while we're at it? lolol
|
Updated to use a single byte buffer instead of sending two separate messages. I used three spaces to keep it properly indented with the system message arrow. I didn't do the inner check skip just because I thought it would further complicate it a bit. |
| body := fmt.Sprintf("Current theme: %s", theme) | ||
| room.Send(message.NewSystemMsg(body, user)) | ||
| var output bytes.Buffer | ||
| fmt.Fprintf(&output, "Current theme: %s%s", theme, message.Newline) |
There was a problem hiding this comment.
Optional nit but IMO:
- fmt.Fprintf(&output, "Current theme: %s%s", theme, message.Newline)
+ fmt.Fprintf(&output, "Current theme: %s" + message.Newline, theme)There was a problem hiding this comment.
Whichever one you prefer I'll go with. I thought using the direct formatting would look better than doing string addition but it's up to you.
There was a problem hiding this comment.
I mean it's kinda the equivalent of writing ("Current theme: %s%s", foo, "\n") which feels weird to me vs ("Current theme: %s\n", foo).
I'll merge it either way, just let me know when it's ready for merging. :)
There was a problem hiding this comment.
I think it's ready. I got nothing else to add.
shazow
changed the title
2 participants
Shows all themes on an empty
/themecall. Not a very friendly way to join all the strings since all the Theme names are locked behindtheme.ID(). It's either do each ID call in each cycle of the loop and add it to a string, or build a list of all the Theme names, then use String.Join to merge them all together and use that. (Third option: have a list of all theme strings somewhere to avoid having to do iteration/list building every time someone calls/theme?)