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

Add .editorconfig file and apply dotnet-format tool #40

Closed
wants to merge 2 commits into from

Conversation

hanabi1224
Copy link

@hanabi1224 hanabi1224 commented Mar 24, 2019

LibVLCSharp/LibVLCSharp.csproj Outdated Show resolved Hide resolved
LibVLCSharp/LibVLCSharp.csproj Outdated Show resolved Hide resolved
@hanabi1224
Copy link
Author

@mfkl comments resolved, plz let me know if there're any other outstanding issues

@mfkl
Copy link
Member

mfkl commented Mar 25, 2019

On your branch, typing dotnet-format gives

Unhandled exception: System.InvalidOperationException: Cannot modify an evaluated object originating in an imported file "C:\Users\Martin\.nuget\packages\msbuild.sdk.extras\1.6.46\Build\Inference.targets".
   at Microsoft.Build.Shared.ErrorUtilities.ThrowInvalidOperation(String resourceName, Object[] args)
   at Microsoft.Build.Shared.ErrorUtilities.VerifyThrowInvalidOperation(Boolean condition, String resourceName, Object arg0)
   at Microsoft.Build.Evaluation.Project.VerifyThrowInvalidOperationNotImported(ProjectRootElement otherXml)
   at Microsoft.Build.Evaluation.Project.RemoveProperty(ProjectProperty property)
   at Microsoft.CodeAnalysis.MSBuild.ProjectFile.GetProjectFileInfosAsync(CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildProjectLoader.Worker.DoOperationAndReportProgressAsync[TResult](ProjectLoadOperation operation, String projectPath, String targetFramework, Func`1 doFunc)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildProjectLoader.Worker.LoadProjectFileInfosAsync(String projectPath, DiagnosticReportingOptions reportingOptions, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildProjectLoader.Worker.LoadProjectInfosFromPathAsync(String projectPath, DiagnosticReportingOptions reportingOptions, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildProjectLoader.Worker.LoadAsync(CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildProjectLoader.LoadSolutionInfoAsync(String solutionFilePath, IProgress`1 progress, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildWorkspace.OpenSolutionAsync(String solutionFilePath, IProgress`1 progress, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Tools.CodeFormatter.CodeFormatter.FormatWorkspaceAsync(ILogger logger, String solutionOrProjectPath, Boolean isSolution, Boolean logAllWorkspaceWarnings, Boolean saveFormattedFiles, CancellationToken cancellationToken) in /_/src/CodeFormatter.cs:line 53
   at Microsoft.CodeAnalysis.Tools.CodeFormatter.Program.Run(String workspace, String verbosity, Boolean dryRun, IConsole console) in /_/src/Program.cs:line 68
   at System.CommandLine.Invocation.CommandHandler.GetResultCodeAsync(Object value, InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.<>c__DisplayClass3_0.<<InvokeAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Invocation.InvocationExtensions.<>c.<<UseParseErrorReporting>b__16_0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass3_0.<<UseVersionOption>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Invocation.InvocationExtensions.<>c.<<UseSuggestDirective>b__7_0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Invocation.InvocationExtensions.<>c.<<UseDebugDirective>b__4_0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Invocation.InvocationExtensions.<>c.<<UseHelp>b__14_0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Invocation.InvocationExtensions.<>c.<<UseParseDirective>b__6_0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Invocation.InvocationExtensions.<>c.<<RegisterWithDotnetSuggest>b__17_0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.CommandLine.Invocation.InvocationExtensions.<>c.<<UseExceptionHandler>b__5_0>d.MoveNext()

@hanabi1224
Copy link
Author

@mfkl dotnet format tool does not seems to support custom build sdk yet, so I manually changed build sdk to Microsoft.NET.SDK before running dotnet format, then change it back : )

@mfkl
Copy link
Member

mfkl commented Mar 25, 2019

if dotnet-format doesn't work out of the box with MSBuild.Sdk.Extras, we will probably find another way to enforce formatting. Modifying this locally each time sounds tiresome.

@hanabi1224
Copy link
Author

Thats why i included .editorconfig as well. Both vs and vs code has extensions that support .editorconfig

@mfkl
Copy link
Member

mfkl commented Mar 25, 2019

Sure, but I still didn't figure out how to use it with the VS extension. Either way, we will want to enforce those formatting rules in the CI so running from CLI is a must.

@hanabi1224
Copy link
Author

hanabi1224 commented Mar 27, 2019

@mfkl Then maybe u can use some alternative cli tools like https://github.com/editorconfig/editorconfig-core-net which seems to be better done in a separate PR

@hanabi1224
Copy link
Author

@mfkl In my VS2017 and VS2019, when .editorconfig file is in place, it can be automatically picked up when performing formatting (Ctrl + K + D) , does that not work for you?

@mfkl
Copy link
Member

mfkl commented Apr 1, 2019

I want this to be enforced on build, either when building with msbuild or by feeding the files to an exe. I'm not counting on contributors to think to do CTRL+K+D before each PR.

Right now, I still don't get how this works. Example:
Setting csharp_space_after_keywords_in_control_flow_statements to false does not output warnings.

@jeremyVignelles
Copy link
Collaborator

@hanabi1224 ; I tried your branch and did see the IDE displaying messages when I wrote int a = 0;.

@mfkl , there must be something wrong with your IDE. Did you enable the Error List window?

Now, we need to define which rules are applied on LibVLCSharp's repo, because the ones you specified are the rules defined by the dotnet-format team, but for example, I tend to disagree with this rule:

dotnet_naming_style.static_field_style.required_prefix = s_

I'd say that a default config (add/new item/editor config (.net)) would be fine for our use, but on the other hand, it would be useful to specify rules like this one (for .sh) : end_of_line = lf (and set that in .gitattributes too)

@jeremyVignelles
Copy link
Collaborator

Adding a .editorconfig is enough for me at that time. We can try locally, and ask for code style fixes as needed.

The automated build failure will come later, but it's not currently supported : dotnet/roslyn#33558

@hanabi1224
Copy link
Author

@jeremyVignelles Regarding s_ prefix thing, I believed it's sth more general within netfx, https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/coding-style.md

@hanabi1224 hanabi1224 closed this Apr 11, 2019
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.

3 participants