add the first set of editor options for visual studio #15020

jmarolf · 2016-11-05T00:43:16Z

Adds the following options

Advanced

editorconfig name	possible values
dotnet_sort_system_directives_first	true , false

Indentation Options

editorconfig name	possible values
csharp_indent_block_contents	true , false
csharp_indent_braces	true , false
csharp_indent_case_contents	true , false
csharp_indent_switch_labels	true , false
csharp_indent_labels	flush_left , no_change , one_less_than_current

New Line Options

editorconfig name	possible values
csharp_new_line_before_open_brace	accessors, anonymous_methods, anonymous_types, control_blocks, events, indexers, lambdas, local_functions, methods, object_collection, properties, types, all, none (or separate with ',')
csharp_new_line_before_else	true , false
csharp_new_line_before_catch	true , false
csharp_new_line_before_finally	true , false
csharp_new_line_before_members_in_object_initializers	true , false
csharp_new_line_before_members_in_anonymous_types	true , false
csharp_new_line_within_query_expression_clauses	true , false

Spacing Options

editorconfig name	possible values
csharp_space_after_cast	true , false
csharp_space_after_colon_in_inheritance_clause	true , false
csharp_space_after_comma	true , false
csharp_space_after_dot	true , false
csharp_space_after_keywords_in_control_flow_statements	true , false
csharp_space_after_semicolon_in_for_statement	true , false
csharp_space_around_binary_operators	none, before_and_after, ignore
csharp_space_around_declaration_statements	ignore, do_not_ignore
csharp_space_before_colon_in_inheritance_clause	true , false
csharp_space_before_comma	true , false
csharp_space_before_dot	true , false
csharp_space_before_semicolon_in_for_statement	true , false
csharp_space_before_open_square_brackets	true , false
csharp_space_between_empty_square_brackets	true , false
csharp_space_between_method_declaration_name_and_open_parenthesis	true , false
csharp_space_between_method_declaration_parameter_list_parentheses	true , false
csharp_space_between_method_declaration_empty_parameter_list_parentheses	true , false
csharp_space_between_method_call_name_and_opening_parenthesis	true , false
csharp_space_between_method_call_parameter_list_parentheses	true , false
csharp_space_between_method_call_empty_parameter_list_parentheses	true , false
csharp_space_between_square_brackets	true , false
csharp_space_between_parentheses	expressions, type_casts, control_flow_statements

Wrapping Options

editorconfig name	possible values
csharp_preserve_single_line_blocks	true , false
csharp_preserve_single_line_statements	true , false

dpoeschl · 2016-11-08T23:18:58Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs

 using Microsoft.CodeAnalysis.Options;
+using System.Linq;


Sort usings. #Pending

dpoeschl · 2016-11-08T23:19:51Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpaceWithinMethodDeclarationParenthesis { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceWithinMethodDeclarationParenthesis), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceWithinMethodDeclarationParenthesis"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_within_method_declaration_parameter_list_parenthesis"),


within the parentheses (plural)? #Pending

dpoeschl · 2016-11-08T23:21:33Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpaceWithinMethodCallParentheses { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceWithinMethodCallParentheses), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceWithinMethodCallParentheses"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_within_method_call_parameter_list_parenthesis"),


Same as above. Parentheses (plural)? #Pending

dpoeschl · 2016-11-08T23:22:47Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpaceWithinExpressionParentheses { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceWithinExpressionParentheses), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceWithinExpressionParentheses"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_within_parentheses", (s) => ParseSpaceOptions(s, SpacingOption.Expressions)),


there's a zillion other csharp_space_within_*_parentheses, so this one without the * is confusing to me. Maybe csharp_space_within_expression_parentheses? #Resolved

dpoeschl · 2016-11-08T23:24:05Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpaceWithinCastParentheses { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceWithinCastParentheses), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceWithinCastParentheses"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_within_parentheses", (s) => ParseSpaceOptions(s, SpacingOption.TypeCasts)),


Oh, this is why. You collapsed several of these into one... Ugh. csharp_space_within_other_parentheses? I don't love it, but I don't love the unqualified version more. #ByDesign

(Feel free to disagree) #ByDesign

dpoeschl · 2016-11-09T00:04:49Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpacesIgnoreAroundVariableDeclaration { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpacesIgnoreAroundVariableDeclaration), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpacesIgnoreAroundVariableDeclaration"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_in_declaration_statements"),


One of these says "spacesignore" and the other one says "space_in" -- is this backwards? #Resolved

dpoeschl · 2016-11-09T00:08:34Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpaceAfterColonInBaseTypeDeclaration { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceAfterColonInBaseTypeDeclaration), defaultValue: true,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceAfterColonInBaseTypeDeclaration"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_after_colon_for_base_or_interface_in_type_declaration"),


Can we do better with the name? I'm not sure how, but maybe there's a way. #Resolved

dpoeschl · 2016-11-09T00:11:02Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> IndentSwitchSection { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(IndentSwitchSection), defaultValue: true,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.IndentSwitchSection"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_indent_case_labels"),


it also indents default: which is probably why they call it "SwitchSection" in the option name. Not sure I care, though. #Resolved

And you matched the text in Tools | Options. It's fine. #Resolved

"indent_switch_labels"

dpoeschl · 2016-11-09T00:16:43Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> WrappingPreserveSingleLine { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(WrappingPreserveSingleLine), defaultValue: true,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.WrappingPreserveSingleLine"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_wrap_block_on_single_line"),


For "spacing" and "indent" the names read nicely. csharp_space_goes_wherever and indent_some_construct. With wrap it doesn't parse so cleanly for me. This makes it seem like you're trying to wrap a block on a single line. I understand that it should be parsed csharp_wrap (pause) block_on_single_line but it's hard to read it that way. #ByDesign

This is a tradeoff between understandable structure and English readability. I don't think I'll change this as I csharp_wrap gives structure to these set of options

In reply to: 87113514 [](ancestors = 87113514)

csharp_wrapping_preserve_block_on_single_line?
csharp_wrapping_keep_block_on_single_line?
csharp_wrapping_do_not_wrap_single_line_block? (my preference)
csharp_wrapping_preserve_single_line_block?

I do not think I will use wrapping as no other option uses that tense.

So, "csharp_wrap_block_on_single_line = true" means "do not wrap a block on a single line? That's super8 confusing.

we need something to indicate that when these are set to true, we do not wrap.

@CyrusNajmabadi

So, "csharp_wrap_block_on_single_line = true" means "do not wrap a block on a single line?

That is not correct

csharp_wrap_block_on_single_line = true

csharp_wrap_block_on_single_line = false

dpoeschl · 2016-11-09T00:20:07Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> NewLineForMembersInAnonymousTypes { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(NewLineForMembersInAnonymousTypes), defaultValue: true,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.NewLineForMembersInAnonymousTypes"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_new_line_before_members_in_anonymouse_types"),


csharp_new_line_before_members_in_anonymouse_types incorrect spelling of anonymous #Resolved

CyrusNajmabadi · 2016-11-10T08:31:55Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+{
+    public static partial class CSharpFormattingOptions
+    {
+        internal static bool ParseParenthesesSpaceOptions(string value, SpacingWithinParenthesesOption parenthesesSpacingOption)


Strange function signature. It's called 'ParseXXX' but returns a bool?

CyrusNajmabadi · 2016-11-10T08:32:51Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+            switch (binaryOperatorSpacingValue)
+            {
+                case "ignore": return BinaryOperatorSpacingOptions.Ignore;
+                case "none": return BinaryOperatorSpacingOptions.Remove;


it bothers me that tehse names don't match.

I'm preferring the names we expose in the IDE over the variable names. It would be a breaking API change the modify the enum names as BinaryOperatorSpacingOptions is a public type.

CyrusNajmabadi · 2016-11-10T08:33:09Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+
+        internal static bool ParseNewLineOption(string value, NewLineOption optionName)
+        {
+            var values = value.Split(',').Select((v) => v.Trim());


parens around (v) not needed

CyrusNajmabadi · 2016-11-10T08:33:17Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+        {
+            var values = value.Split(',').Select((v) => v.Trim());
+
+            if (values.Any((v) => v == "all"))


CyrusNajmabadi · 2016-11-10T08:35:16Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpaceBetweenEmptyMethodCallParentheses { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceBetweenEmptyMethodCallParentheses), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceBetweenEmptyMethodCallParentheses"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_within_method_call_empty_parameter_list_parentheses"),


i'm torn on how i feel about this. So Roslyn calls these 'Invocations' not method-calls. But the old IDE called the method calls. I'm wondering if we should unify on roslyn naming as we move forward. A core reason for this is that we often have confusion around what the option name is and what it actually controls with our new APi.

I think calls is fine - it's throughout the IDE (Call Hierachy, Call stack, etc). I see no reason to change the user facing term to the API term.

CyrusNajmabadi · 2016-11-10T08:38:15Z

src/Workspaces/CSharpTest/Formatting/EditorConfigOptionParserTests.cs

+        [InlineData("type_casts, expressions, , ", SpacingWithinParenthesesOption.TypeCasts)]
+        [InlineData("expressions ,  ,  , control_flow_statements", SpacingWithinParenthesesOption.ControlFlowStatements)]
+        [InlineData("expressions ,  ,  , control_flow_statements", SpacingWithinParenthesesOption.Expressions)]
+        [InlineData(",  ,  , control_flow_statements", SpacingWithinParenthesesOption.ControlFlowStatements)]


witchcraft!

Alternatively, get rid of a bunch of brackets and just put , between the atrributes

CyrusNajmabadi · 2016-11-11T05:33:31Z

We can deprecate the old names, and add the new names to the enum, with the same values as the older ones.

Pilchie

So excited to have this.

Should we re-base to dev15-rc2?

Pilchie · 2016-11-11T19:58:44Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpaceBetweenEmptyMethodCallParentheses { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceBetweenEmptyMethodCallParentheses), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceBetweenEmptyMethodCallParentheses"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_within_method_call_empty_parameter_list_parentheses"),


I think calls is fine - it's throughout the IDE (Call Hierachy, Call stack, etc). I see no reason to change the user facing term to the API term.

Pilchie · 2016-11-11T20:00:02Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs

    {
        public static Option<bool> SpacingAfterMethodDeclarationName { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpacingAfterMethodDeclarationName), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpacingAfterMethodDeclarationName"));
+            storageLocations: new OptionStorageLocation[] {


Nit: Can you use implicit array creations here with just new [] {...?

EditorConfigStorageLocation and RoamingProfileStorageLocation are both derived from OptionStorageLocation but implicit array creation cannot infer OptionStorageLocation

That seems unexpected/crappy. :-/

I could cast them explcitly

public static Option<bool> SpaceWithinMethodDeclarationParenthesis { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpaceWithinMethodDeclarationParenthesis), defaultValue: false, storageLocations: new [] { (OptionStorageLocation)new EditorConfigStorageLocation("csharp_space_within_method_declaration_parameter_list_parentheses"), (OptionStorageLocation)new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpaceWithinMethodDeclarationParenthesis")});

No, this is fine if it's the best the compiler can do.

Pilchie · 2016-11-11T20:01:58Z

src/Workspaces/CSharpTest/Formatting/EditorConfigOptionParserTests.cs

+        [InlineData("type_casts, expressions, , ", SpacingWithinParenthesesOption.TypeCasts)]
+        [InlineData("expressions ,  ,  , control_flow_statements", SpacingWithinParenthesesOption.ControlFlowStatements)]
+        [InlineData("expressions ,  ,  , control_flow_statements", SpacingWithinParenthesesOption.Expressions)]
+        [InlineData(",  ,  , control_flow_statements", SpacingWithinParenthesesOption.ControlFlowStatements)]


Alternatively, get rid of a bunch of brackets and just put , between the atrributes

jasonmalinowski

Nitpicks only.

jasonmalinowski · 2016-11-11T20:30:26Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+{
+    public static partial class CSharpFormattingOptions
+    {
+        internal static bool DetermindIfSpaceOptionIsSet(string value, SpacingWithinParenthesesOption parenthesesSpacingOption)


Typo: "Determind".

jasonmalinowski · 2016-11-11T20:31:41Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+    {
+        internal static bool DetermindIfSpaceOptionIsSet(string value, SpacingWithinParenthesesOption parenthesesSpacingOption)
+            => (from v in value.Split(',').Select(v => v.Trim())
+                let option = ConvertToSpacingOption(v)


You could just select ConvertToSpacingOption directly and the .Any() would just handle the null case for you, right?

jasonmalinowski · 2016-11-11T20:32:24Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+            }
+        }
+
+        internal static object ParseEditorConfigSpacingAroundBinaryOperator(string binaryOperatorSpacingValue)


Why weakly typed here?

EditorConfigStorageLocation requires Func<string, object>

Error CS0407 'BinaryOperatorSpacingOptions CSharpFormattingOptions.ParseEditorConfigSpacingAroundBinaryOperator(string)' has the wrong return type

I'd prefer the return type still be: "BinaryOperatorSpacingOptions ". But htat hte consumer does hte conversion with a lambda.

jasonmalinowski · 2016-11-11T20:32:40Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+            }
+        }
+
+        internal static object ParseEditorConfigLabelPositioning(string lableIndentationValue)


Typo: "lable"

jasonmalinowski · 2016-11-11T20:33:13Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+        {
+            var values = value.Split(',').Select(v => v.Trim());
+
+            if (values.Any(v => v == "all"))


jasonmalinowski · 2016-11-11T20:33:45Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.Parsers.cs

+            }
+
+            return (from v in values
+                    let option = ConvertToNewLineOption(v)


Can we unify this code with the other Determine* functions?

jasonmalinowski · 2016-11-11T20:34:32Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs

    {
        public static Option<bool> SpacingAfterMethodDeclarationName { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpacingAfterMethodDeclarationName), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpacingAfterMethodDeclarationName"));
+            storageLocations: new OptionStorageLocation[] {


That seems unexpected/crappy. :-/

jasonmalinowski · 2016-11-11T20:36:53Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs


        public static Option<bool> SpacesIgnoreAroundVariableDeclaration { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpacesIgnoreAroundVariableDeclaration), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpacesIgnoreAroundVariableDeclaration"));
+            storageLocations: new OptionStorageLocation[] {
+                new EditorConfigStorageLocation("csharp_space_ignore_around_declaration_statements"),


Is this one better done as csharp_space_around_declaration_statements = ignore or something? It feels like having the checkboxes that say "don't do this".

jasonmalinowski · 2016-11-11T20:41:38Z

src/Workspaces/CSharpTest/Formatting/EditorConfigOptionParserTests.cs

+        [InlineData("one_less_than_current", LabelPositionOptions.OneLess)]
+        static void TestParseEditorConfigLabelPositioningTrue(string value, LabelPositionOptions expectedValue)
+        {
+            Assert.True((LabelPositionOptions)ParseEditorConfigLabelPositioning(value) == expectedValue,


Yep, let's make these methods more strongly typed.

jmarolf · 2016-11-12T01:53:43Z

@Pilchie @MattGertz @natidea I've re-targeted the PR against RC.2, please let me know it we want to take this.

Customer scenario

The customer can now set visual studio settings in editorconfig files that allow different editor options to be specified per folder.

Bugs this fixes: (either VSO or GitHub links)

Support enforcing "Code Style" in the IDE #7071

Workarounds

The user can manually change their settings each time they open a solution file, but cannot have different settings from different folders within that solution.

Risk

This uses the editorconfig API from the Visual Studio Platfrom team, it does not introduce code into roslyn other than consuming that API and handing it to the options service

Performance impact

When folders are quieried is set by the Visual Studio Platfrom team which has done the work to not requery unless their are file changes. Perf impact should be Low

Is this a regression from a previous update?

No.

Root cause analysis: how did we miss it? What tests are we adding to guard against it in the future?

This is not a regression. It is new feature work.

How was the bug found? (E.g. customer reported it vs. ad hoc testing)

This is not a bug. It is new feature work.

jmarolf · 2016-11-12T01:56:04Z

@CyrusNajmabadi

We can deprecate the old names, and add the new names to the enum, with the same values as the older ones.

We certainly can. I would need to compat council to sign off in that case

MattGertz · 2016-11-12T05:13:31Z

Approved pending @dpoeschl agreement re:his requested changes.

Pilchie · 2016-11-13T01:03:01Z

src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs

    {
        public static Option<bool> SpacingAfterMethodDeclarationName { get; } = new Option<bool>(nameof(CSharpFormattingOptions), nameof(SpacingAfterMethodDeclarationName), defaultValue: false,
-            storageLocations: new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.SpacingAfterMethodDeclarationName"));
+            storageLocations: new OptionStorageLocation[] {


No, this is fine if it's the best the compiler can do.

dpoeschl

Thanks for all the changes. Looking good.

CyrusNajmabadi · 2016-11-16T00:06:53Z

csharp_space_after_colon_for_base_or_interface_in_type_declaration => csharp_space_after_colon_in_inheritance_clause

CyrusNajmabadi · 2016-11-16T00:10:41Z

csharp_space_between_method_declaration_name_and_open_parentheses => csharp_space_between_method_declaration_name_and_open_parenthesis
csharp_space_between_method_call_name_and_opening_parentheses => csharp_space_between_method_call_name_and_open_parenthesis

csharp_space_around_operators => csharp_space_around_binary_operators ?

csharp_keep_block_on_single_line => csharp_preserve_single_line_blocks
csharp_keep_statement_and_member_declarations_on_same_line => csharp_preserve_single_line_statements

csharp_preserve_single_line_member_declarations

CyrusNajmabadi · 2016-11-16T00:13:50Z

not suggesitng, just preserving:
csharp_indent_label_one_less_than_current_level
csharp_indent_label_do_not_change
csharp_indent_label_to_zero_column

also: mouse.

jmarolf · 2016-11-16T01:09:07Z

@CyrusNajmabadi Updated with your suggestions

for csharp_indent_labels I changed the possible options to be flush_left , no_change , one_less_than_current

CyrusNajmabadi · 2016-11-16T08:26:52Z

csharp_new_line_before_open_brace
possible values has event_accessor, but not property_accessors?

Spacing Options
can we sort these by name?

Usages of both between and within. Seems synonymous.

What happened to csharp_preserve_single_line_member_declarations?

jmarolf · 2016-11-16T21:22:23Z

@CyrusNajmabadi

csharp_new_line_before_open_brace possible values has event_accessor, but not property_accessors?

Options updated

Spacing Options can we sort these by name?

Done

Usages of both between and within. Seems synonymous.

Use within everywhere

What happened to csharp_preserve_single_line_member_declarations?

I don't know, I've never used that as a name for anything and I can't remember why you wrote it in the previous comment. Do you recall what this was about?

CyrusNajmabadi · 2016-11-16T21:23:59Z

csharp_space_around_declaration_statements

Is this about the space in a field/location declaration? Or does it refer to more than that?

CyrusNajmabadi · 2016-11-16T21:25:24Z

Overall i like it. I feel like i like "between" versus "within". I think it reads better. But if you feel "within" is better, i'll defer.

jmarolf · 2016-11-17T00:58:39Z

@CyrusNajmabadi

Is this about the space in a field/location declaration? Or does it refer to more than that?

It should just be about field /local declarations. I've updated the possible values to be more clear

editorconfig name	possible values
csharp_space_around_declaration_statements	ignore, do_not_ignore

I like "between" versus "within". I think it reads better. But if you feel "within" is better, i'll defer.

I only picked within because between was one character longer. I will happily switch it back for better readability

CyrusNajmabadi · 2016-11-17T02:53:52Z

👍

ygoe · 2017-04-17T19:21:54Z

How is csharp_space_between_parentheses supposed to be used? It looks like the values are actually options that need a value like true or false. This design seems broken. I couldn't follow the code far enough to find the meaning.

jmarolf · 2017-04-17T23:45:50Z

You can specify expressions, type_casts, or control_flow_statements separated by a comma.

If expressions is present it means "insert space within parenthesis of expressions" like so:

var x = 3;
var y = 4;
var z = ( x * y ) - ( ( y - x ) * 3 );

If expressions is not present your code will be formatted like:

var x = 3;
var y = 4;
var z = (x * y) - ((y - x) * 3);

If type_casts is present it means "insert space within parenthesis of type casts" like so:

int y = ( int )x;

If type_casts is not present your code will be formatted like:

int y = (int)x;

If control_flow_statements is present it means "insert space within parenthesis of control flow statements" like so:

for( int i; i < x; i++ )
{
}

If control_flow_statements is not present your code will be formatted like:

for(int i; i < x; i++)
{
}

An example of all three specified in an editorconfig file would be:

# Code files
[*.cs]
csharp_space_between_parentheses = expressions,type_casts,control_flow_statements

ygoe · 2017-04-18T07:14:17Z

I didn't know multiple values were allowed, this isn't obvious.

What's the value for none of these? Simply csharp_space_between_parentheses =?

jmarolf · 2017-04-18T16:08:23Z

@ygoe do not specify the option at all if you do not want spaces added in any of these scenarios.

jmarolf · 2017-04-18T16:11:22Z

I didn't know multiple values were allowed, this isn't obvious.

Some of these options may be removed at some point due to low to no useage, which is why we haven't documented several of them. Is this an option that you want to turn on?

ygoe · 2017-04-18T20:16:17Z

My point is to explicitly override whatever the IDE is configured to. Not specifying the option means "no intent" which is different from "intent to not use spaces". All other options let me specify "true", "false" or nothing at all. This one is different.

dnfclas added the cla-already-signed label Nov 5, 2016

jmarolf added Area-Analyzers Area-IDE and removed Area-Analyzers labels Nov 7, 2016

brettfo approved these changes Nov 8, 2016

View reviewed changes

dpoeschl suggested changes Nov 9, 2016

View reviewed changes

jmarolf force-pushed the feature/editorconfig-general-editor-options branch 4 times, most recently from 6b1fabe to c20c007 Compare November 10, 2016 00:37

CyrusNajmabadi reviewed Nov 10, 2016

View reviewed changes

jmarolf force-pushed the feature/editorconfig-general-editor-options branch from c20c007 to 2654a6c Compare November 11, 2016 03:46

Pilchie approved these changes Nov 11, 2016

View reviewed changes

jmarolf force-pushed the feature/editorconfig-general-editor-options branch from 2654a6c to 3f8fc56 Compare November 11, 2016 20:33

jmarolf changed the base branch from master to dev15-rc2 November 11, 2016 20:33

jmarolf force-pushed the feature/editorconfig-general-editor-options branch from 3f8fc56 to 4769c05 Compare November 11, 2016 20:35

jasonmalinowski approved these changes Nov 11, 2016

View reviewed changes

MattGertz added the Approved to merge label Nov 12, 2016

Pilchie approved these changes Nov 13, 2016

View reviewed changes

jmarolf force-pushed the feature/editorconfig-general-editor-options branch from 33fc8a6 to 4c9163f Compare November 14, 2016 20:27

dpoeschl approved these changes Nov 14, 2016

View reviewed changes

CyrusNajmabadi closed this Nov 16, 2016

CyrusNajmabadi reopened this Nov 16, 2016

dnfclas added the cla-already-signed label Nov 16, 2016

jmarolf force-pushed the feature/editorconfig-general-editor-options branch from 6ab5f8e to d46a349 Compare November 16, 2016 01:07

add visual studio editor options for editorconfig

c0cd291

jmarolf force-pushed the feature/editorconfig-general-editor-options branch from 1a3dd29 to c0cd291 Compare November 17, 2016 05:09

jmarolf merged commit 8da74ba into dotnet:dev15-rc2 Nov 17, 2016

sharwell mentioned this pull request Dec 5, 2016

Proposal: Configurable Spacings DotNetAnalyzers/StyleCopAnalyzers#2233

Open

jmarolf mentioned this pull request Dec 8, 2016

Added .editorconfig files for Roslyn project #15620

Merged

tannergooding mentioned this pull request Mar 10, 2017

Improved Asin and ACos dotnet/corefx#16484

Merged

RehanSaeed mentioned this pull request Mar 17, 2017

Missing Settings madskristensen/EditorConfigLanguage#10

Open

jmarolf deleted the feature/editorconfig-general-editor-options branch January 27, 2020 16:59

This was referenced Jan 31, 2020

The .editorconfig should be updated to support the code style options made available in VS2017 dotnet/runtime#7602

Closed

The .editorconfig should be updated to support the code style options made available in VS2017 dotnet/runtime#20563

Closed

add the first set of editor options for visual studio #15020

add the first set of editor options for visual studio #15020

Conversation

jmarolf commented Nov 5, 2016 • edited Loading

Advanced

Indentation Options

New Line Options

Spacing Options

Wrapping Options

dpoeschl Nov 8, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 8, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 8, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 8, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 8, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 9, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 9, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 9, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 9, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

dpoeschl Nov 9, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpoeschl Nov 9, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi Nov 10, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarolf Nov 14, 2016 • edited Loading

Choose a reason for hiding this comment

dpoeschl Nov 9, 2016 • edited by jmarolf Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Nov 11, 2016

Pilchie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonmalinowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarolf commented Nov 12, 2016

Customer scenario

Bugs this fixes: (either VSO or GitHub links)

Workarounds

Risk

Performance impact

jmarolf commented Nov 5, 2016 •

edited

Loading

dpoeschl Nov 8, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 8, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 8, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 8, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 8, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 9, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 9, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 9, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 9, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 9, 2016 •

edited by jmarolf

Loading

dpoeschl Nov 9, 2016 •

edited by jmarolf

Loading

CyrusNajmabadi Nov 10, 2016 •

edited

Loading

jmarolf Nov 14, 2016 •

edited

Loading

dpoeschl Nov 9, 2016 •

edited by jmarolf

Loading