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

added Encoding property for consoleTarget + ColorConsoleTarget #692

Merged
7 commits merged into from
May 11, 2015
Merged

added Encoding property for consoleTarget + ColorConsoleTarget #692

7 commits merged into from
May 11, 2015

Conversation

304NotModified
Copy link
Member

Closes #676

No unit tests added as it is a small addition.

The property has a side effect (static property on Console), but I think we can live with that.

@304NotModified 304NotModified added this to the 4.0 milestone May 5, 2015
@ghost
Copy link

ghost commented May 10, 2015

Looks good, but I think the using should be moved inside the !IF SILVERLIGHT block like the rest of them

@304NotModified
Copy link
Member Author

I will update my PR.

@304NotModified
Copy link
Member Author

@Xharze I don't understand your comment I think:

  • ColoredConsoleTarget is already in #if !SILVERLIGHT
  • Encoding is available in Silverlight? (see FileTarget) - for ConsoleTarget

@@ -31,6 +31,8 @@
// THE POSSIBILITY OF SUCH DAMAGE.
//

using System.Text;
Copy link

Choose a reason for hiding this comment

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

Move inside #if !SILVERLIGHT

@ghost
Copy link

ghost commented May 10, 2015

I've tried to add as a line comment :)

@304NotModified
Copy link
Member Author

Thanks, I fixed it :)

Including also some doc/newlines stuff

ghost pushed a commit that referenced this pull request May 11, 2015
added Encoding property for consoleTarget + ColorConsoleTarget
@ghost ghost merged commit 6892468 into NLog:master May 11, 2015
@304NotModified 304NotModified deleted the encoding-console branch August 15, 2015 20:48
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants