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

allow loglevel to be passed in as an argument #714

Merged
merged 4 commits into from
Jan 11, 2017

Conversation

filipw
Copy link
Member

@filipw filipw commented Dec 20, 2016

This PR introduces a new startup argument -loglevel {value}, which allows OmniSharp logging level to be configured from outside.

At the moment there is only a -v switch which toggles between information and debug levels, but there is no way to set up more granular logging levels. For now I have not removed the -v switch yet, so as to not break any existing use case.

The PR will allow adding this feature dotnet/vscode-csharp#993 to the editors.

@filipw filipw changed the title allow loglevel to passed as argument allow loglevel to be passed as argument Dec 20, 2016
@willl willl changed the title allow loglevel to be passed as argument allow loglevel to be passed as an argument Dec 20, 2016
@willl willl changed the title allow loglevel to be passed as an argument allow loglevel to be passed in as an argument Dec 20, 2016
@@ -48,6 +48,15 @@ public static void Main(string[] args)
enumerator.MoveNext();
serverPort = int.Parse((string)enumerator.Current);
}
else if (arg.ToLowerInvariant() == "-loglevel")
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to use string.Equals(arg, "-loglevel", StringComparison.OrdinalIgnoreCase) rather than ToLowerInvariant() here?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

Copy link
Member

@willl willl left a comment

Choose a reason for hiding this comment

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

I like it. 👍

We should probably make other longer arguments, such as hostPID to also allow arguments to be passed in with any case.

Copy link
Member

@david-driscoll david-driscoll left a comment

Choose a reason for hiding this comment

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

LGTM

Added @DustinCampbell in case he had any arguments, but it looks fine to me. -v will always override -loglevel which makes sense.

@DustinCampbell
Copy link
Contributor

Nope. No complaints. Although, we should consolidate these arguments at some point.

@filipw filipw merged commit 03efdad into OmniSharp:dev Jan 11, 2017
@filipw
Copy link
Member Author

filipw commented Jan 11, 2017

great. let's punch this in

@filipw filipw deleted the feature/loglevel branch January 11, 2017 15:21
@filipw filipw mentioned this pull request Jan 28, 2017
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