Skip to content

Add ability to toggle showing/hiding channel that a message was sent …#45

Merged
bbbscarter merged 3 commits into
bbbscarter:masterfrom
astruyk:ChannelDisplay
Mar 1, 2018
Merged

Add ability to toggle showing/hiding channel that a message was sent …#45
bbbscarter merged 3 commits into
bbbscarter:masterfrom
astruyk:ChannelDisplay

Conversation

@astruyk

@astruyk astruyk commented Feb 27, 2018

Copy link
Copy Markdown
Contributor

…to in the log output.

(opening new pull request with branched version, as request in #42 )

@bbbscarter bbbscarter left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice! Very happy to take this with some minor modifications.

showTimes ? log.GetRelativeTimeStampAsString() : "",
showChannels || showTimes ? " : " : "",
showMessage
);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm finding the conditional operators hard to parse here. Could we refactor into something like:
var channelMessage = showChannels ? String.Format("[{0}] : ", log.Channel : "";
var timeMessage = showTime ? log.GetRelativeTimeStampAsString() + " : " : "";
showMessage = String.Format("{0}{1}{2}". channelMessage, timeMessage, showMessage);

…ity. Add comment explaining expected output for reference.
…n if display channels setting is enabled). Prevents '[]' from showing up for non-channel based logs.
@bbbscarter

Copy link
Copy Markdown
Owner

Super - thanks!

@bbbscarter bbbscarter merged commit e0189c9 into bbbscarter:master Mar 1, 2018
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.

2 participants