833 Allow log level to be set in debug mode.#872
Conversation
|
Thanks @jamiehumphries. I think this is a much better solution! |
There was a problem hiding this comment.
Is this a replication of the logic in ArgumentHandler? I.e. the resolution of the log level and debug switch should already have been done by the time this code is run, no?
There was a problem hiding this comment.
Correct, but I left this here for a couple of reasons:
- I had to do something to convert from a nullable
LogLevel?to a non-nullLogLevelso that it could be passed as a parameter below. Just doingcommandArgs.LogLevel.Valuefelt a bit wrong and also gave aPossible 'System.InvalidOperationException'warning. - Since this method and class are public, it didn't feel right to assume that it would only ever be called with args that have been sanitized in
ArgumentHandler.
I also considered only doing the logic here and not in ArgumentHandler, but again that felt wrong because the ArgumentHandler should output the args in their expected final form.
The only other option that I could think of was having two classes along the lines of PreParsingScriptCsArgs and ParsedScriptCsArgs (with better names!) which had a LogLevel? and a LogLevel respectively, but I felt that may have been overkill!
There was a problem hiding this comment.
OK, fair points. How about a compromise: this line simply defaults to info level when null, but the resolution of log level and debug switch is left in ArgumentHandler where it belongs? I just want to avoid the duplication.
There was a problem hiding this comment.
Yep fine. I'll change that and put it back up.
|
@adamralph, yeah I agree that this is better. The slight benifit to #871 (the reason I did it that way to begin with) is that it makes no changes at all to the public interface of the library, whereas this turns a public |
|
Awesome, thanks! |
833 Allow log level to be set in debug mode.
|
@jamiehumphries you've been added to https://github.com/scriptcs/scriptcs/wiki/Contributors 🏆 This will be part of the 0.12 release. |
|
Thanks @adamralph 😄 |
|
This is nice! 😁👍 |
Fixes #833 by allowing any log level to be set in debug mode. If this is accepted then it makes #871 obsolete.
@Romoku, do you want to see if this does what you hope it will? You should run with
-D -loglevel infoorerrorif you do not want to see all of thedebugoutput.