dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.92k stars 4.02k forks source link

Smart indenting possible issue. #4351

Open KakCAT opened 9 years ago

KakCAT commented 9 years ago

Hello,

I don't know if this is an issue or not, because smart indenting is not well defined anywhere (what is clear is that behaviour is different from previous VS versions) In my coding style I usually remove the 2 initial nesting levels because I just feel they bring no information at all and they just waster screen space.

In example, my "typical" code looks like this

vs2013_image1

As said, no tabs after namespace nor class nor function definition. Everything inside "void bar" is smart-indenting style compliant and had no problems nor complaint until VS2015 appeared.

Now ,I continue typing at the cursor. I press 'enter', then myOtherFunc(); and enter again. This is the result

vs2013_image2

Smart tabbing puts my line below the previous line, and the cursor position too. I don't know if it's correct or not, but it's the way it works (and works well for me)

However in VS2015 this has turned hell. When typing myOtherFunc, VS2015 autoindents it as if the namespace and class would be indented as if I'd have obeyed the smart indenting method. This is the result in the current version of VS2015:

vs2015_image2

The indenter knows that if I had followed the guidelines, "call4()" should add 3 tabs, so it adds 3 tabs, no matter if the last '{' is tabbed with a different amount of tabs So, typing the function autoindents EVERY line with 3 tabs. So, even if I "fix" the last typed line, the next line will be again tabbed 4 times and will have to fix it again.

So, my question is... is this the correct expected behaviour and VS2013 and previous versions were wrong? Is this a bug to be fixed in the future, or I must start changing thousands of files to match the imposed VS2015 smart indenting, or get used to block indenting again?

Thanks!

note: configuration for all my test in VS2013 and VS2015 are "Reset to defaults", then selected "Indenting Smart", "Keep tabs" and then disable all the "Automatically format..." in options: Text Editor/C#/Formatting

Pilchie commented 9 years ago

This is a known design change in the way the formatting engine works. @heejaechang and/or @basoundr can probably explain more of the details, but it's unfortunately something would be difficult to change in the current implementation of formatting.

heejaechang commented 9 years ago

@KakCAT if you are disabling all auto formatting any reason you keep smart indenting rather than using block indenting? block indenting was workaround in mind when I made this behavior change.

new format engine in vs 2015 share core rules between auto formatter and intender so indenter will put caret at the position where formatter would have put code. that is why the caret goes where you described above.

but I think we should leave this issue as suggestion and see how many upvote we get. and if there are enough people who want old behavior back (meaning block indenting is not good enough workaround) then, we could spend some time to bring old behavior (or similar) back.

KakCAT commented 9 years ago

Hi @Pilchie , @heejaechang

well, in previous versions of Visual Studio I've always considered that smart indenting and autoformatting are two very different features. I hate when my code is modified automatically, but I love when VS puts the next line in the place it should be so I don't have to lose time with tab-formatting. I understand that in order to move forward, both systems have had to be merged.

To me, smart indenting is (was) "putting the caret in the same position as the previous line, except if the last line ends with '{', in which case add an extra TAB, or your current line is a '}', in which case you remove the extra TAB ". As you say, this is quite similar to block indenting, except for the extra TABs.

Right now I'll be far more confortable with the block indenting (and adding that TAB each time I need it) so there's no big problem in using it (despite the fact that there are several places I'm already reporting where the autoformat/smart indenting "attacks" even when disabled and using block indenting). I just wanted to know for sure what would be the outcome of this behaviour.

The alternative is changing all the files I've been working in the last 4 years and adapt to the 'forced' indenting style, which I honestly would prefer to avoid. :smile:

p.s. maybe the easiest way to fix this problem would be adding to block indent those 'slightly smart' features (as non default behaviour) like adding a TAB and removing TAB when '{' and '}' are present, but I don't know if this is a feature that can be requested as lightly as I'm doing ;)

basoundr commented 9 years ago

As @pilchie mentioned, this is a known design change in the formatting engine. When "smart-indenting" is selected, the indentation of the new line is decided on the syntatic structure of the code rather than how the code is written . The assumption here is, if the user follows their own custom formatting style then the user wont need smart indenting(because the user has custom formatting style)

Given that we supported the custom formatting style in VS2013 mentioned in this issue, it would be nice for the users to continue using this behavior in VS2015. One approach could be, we introduce 2 options(or 1 option), IndentNamespaceContent and IndentTypeContent which determine if the namespace content and type content should be indented or not respectively. These 2 options will live alongside other Indent style options. The drawback of the approach is, we are adding 2 more options to already existing bunch of options.

The above approach will not be a general solution to all kind of custom tabbing formatting style set by the user but it should address majority of the cases.

KakCAT commented 9 years ago

Hi @basoundr ,

I don't think adding a switch for namespace and type would be effective. In example, I use the coding style described in this thread for all my projects, but I also have to edit Open Source libs which use the common C# indenting style.

Adding this switch would fix my projects, but would transfer the problem to the OS edited projects, unless I switch back and forth the settings every time I load a project (or the setting is stored by project instead of per language, which afaik is not being done now)

HaloFour commented 9 years ago

Is this at all related to the lack of auto-indentation when writing out fluent API within a lambda? This is something incredibly common in frameworks like Entity Framework and since I've upgraded to Visual Studio 2015 the experience has become quite painful. The first fluent call, outside of the lambda, auto-indents fine, but all of the inner fluent calls automatically indent to the same position as the first indentation and I have to adjust them manually.

The following is example code for Entity Framework 6.x Code First to map an entity to stored procedures:

modelBuilder.Entity<Customer>()
    .MapToStoredProcedures(config => config
        .Insert(insert => insert
            .HasName("add_customer", "dbo")
            .Parameter(c => c.Name, "name")
            .Parameter(c => c.Address.Street1, "street1")
            .Parameter(c => c.Address.Street2, "street2")
            .Parameter(c => c.Address.City, "city")
            .Parameter(c => c.Address.State, "state")
            .Parameter(c => c.Address.PostalCode, "zip")
            .Result(c => c.Id, "customer_id"))
        .Update(update => update
            .HasName("set_customer", "dbo")
            .Parameter(c => c.Id, "id")
            .Parameter(c => c.Name, "name")
            .Parameter(c => c.Address.Street1, "street1")
            .Parameter(c => c.Address.Street2, "street2")
            .Parameter(c => c.Address.City, "city")
            .Parameter(c => c.Address.State, "state")
            .Parameter(c => c.Address.PostalCode, "zip"))
        .Delete(delete => delete
            .HasName("rem_customer", "dbo")
            .Parameter(c => c.Id, "id")));
sgranade commented 8 years ago

I just stumbled on this issue in looking to move to VS2015, as our house style involves not indenting namespaces. In @basoundr's suggestion above, we'd set IndentNamespaceContent to false.

zvilius commented 8 years ago

I will add a voice in support of the OP's point of view on smart indenting in VS2015. I have the exact same issue, and the VS2015 editor is really driving me crazy. I have the exact same rationale for eliminating the first two columns of whitespace in most of my code, and the VS2013 and earlier editors allowed me to do that with little fuss.

I've tried switching to block indenting, but the editor is still fighting me. For me this is a big step backward, I'm sorry to say.

Mark Zvilius

rickpmartin commented 8 years ago

Same issue here also, we also have mixed cases where the indentation should and should not be applied. The behavior in VS2013 was great for both cases, whereas now this is quite painful.

vik3000 commented 8 years ago

This issue prevents us from moving to VS2015. We have to maintain code of various origin and some source files simply cannot be edited in VS2015 without resorting to fully manual indentation. Example of such a file, which could be result of modification by two developers with different settings: problemwithindentationinvs2015-3 I have created bug report for this here. However, it has been promptly closed with note saying, that the issue is being tracked on this thread on GitHub. And on this thread there is silence from MS for almost a year...

tunger commented 8 years ago

This prevents me as well from moving to VS2015 for specific projects. One has lots of old code files with different indent size (2). VS2015 only wants to work with the current setting (4), no matter the indent size in the code file. In VS2013 this works well, with VS2015 I would need to reformat the whole file, or manually adjust the indent for all changed/added lines - which is impractical. Here is a sample code which illustrates the issue. Settings: smart indenting, tab & indent size 4, insert spaces. VS2015 Update 2.

using System;

namespace Foo
{
  public class Bar
  {
    public void Method()
    {
            // New line starts here
    }

        public void NewlyAdded()
        {
        } // Typing closing brace doesn't align with other methods

    public void LotsOfCode()
    {
      Console.WriteLine();
      Console.WriteLine();
      Console.WriteLine();

      if (true)
      {
        Console.WriteLine();

        if (true)
        {
          Console.WriteLine();

                    // New line starts here
        }

        Console.WriteLine();
      }

      if (true)
      {
        Console.WriteLine();

        if (true)
        {
                    // Pasting this line results in this, intending surrounding code
                    // Same happens when refactoring
                }

                Console.WriteLine();
      }

      Console.WriteLine();
    }
  }
}
heejaechang commented 8 years ago

is block indenting doesn't work for you? what you described seems quite similar to what block indenting does.

if we provide a way to set tab/indent size for each solution/project/directory, will that solve your problem? (rather than current VS wide setting)

tunger commented 8 years ago

I tried that, and you are right, block indenting sets the cursor to the right place for a new line. However, when the formatter runs (semicolon, brace), it indents in the line in the size like configured in the settings. So unfortunately that doesn't solve the issue, the outcome is the same when working with a file with different indent size than configured in the editor settings.

I don't think a setting per solution/project/directory is a solution, as we may have files with different indent sizes in a directory. Best would be like in VS2013, where the formatter/indentation would recognize the indent for the current code block, and apply the indentation size from there - rather than only multiplying indent size by the number of code blocks.

heejaechang commented 8 years ago

hmmm that probably only work for the very first indentation, but what happen for the inner one?

void Method() { [ this indentation] }

are you fine that to be following settings in the editor?

...

so, if you can set indentation per file, would that solve it?

...

what you described of old behavior only works some times. if you turned on auto formatting and have arbitrary indentation, whenever formatter runs there is a chance of formatter changing all your indentation.

example will be auto formatting namespace, or method body.

vik3000 commented 8 years ago

Hi, thanks for returning to this still very actual topic. No, setting indentation per file would not solve the problem either. Indentation can be mixed even within single file (eg. as a result of modification by developers with different indentation setting). Or the indentation may not follow the rules you assume (like omitting the indentation after namespace block). Please see the examples earlier in this thread.

The block indentation doesn't work in these scenarios (as mentioned by Tunger, cursor is placed correctly, but after pressing semicolon etc. the line gets shifted).

tunger commented 8 years ago

@heejaechang Regarding your question. When using block indentating with indent size 4, and editing an indent size 2 document, this happens.

Before typing semicolon:

namespace Foo
{
  public class Bar
  {
    public void Method()
    {
    // Cursor is placed here
    Console.WriteLine()
    }
  }
}

After typing semicolon:

namespace Foo
{
  public class Bar
  {
    public void Method()
    {
            // This comment was shifted as well
            Console.WriteLine();
            // -> method body is shifted to 4*3=12 spaces
    }
  }
}

You would need to revert to manual indenting, or reformat whole document.

As for smart indenting, yes, you are right, formatting namespace or class would format the whole content of it. The main difference is, that previously, the existing indentation was taken, and from there the indent size from the settings is added. Now, the existing indentation is ignored.

Using above example, Console.WriteLine(); would be put at the following indentation:

I would agree with vik3000. Settings per file seems like a workaround for the issue and does not solve all cases as outlined by vik3000. It would also be a great administrative effort of configuring, when thinking of that large project of mine which has lots of code files with different indent size. And even then, I myself don't want to use that small indent for new code in these files, which I would then be forced to use ;)

heejaechang commented 8 years ago

@dotnet/roslyn-ide should we do this?

I think we could do what users want by changing this a bit. http://source.roslyn.io/#Microsoft.CodeAnalysis.Workspaces/Formatting/BottomUpBaseIndentationFinder.cs,24

CyrusNajmabadi commented 8 years ago

Here's my take after reading through the thread:

  1. We should probably add a "Do not indent namespace content" option. It seems cheap and easy to do, and i'm sympathetic to people who want to code such that namespaces are flush left to avoid lots of blank space.
  2. I don't think it's necessary to support the concept of different indentation levels in a file. Indeed, i think we're going to push in the opposite direction with our .editor_config work. I don't think it's beneficial or desirable for a single file to have a multitude of indentations in it. Such code ends up being difficult for everyone to then edit. Because of .editor_config though, it's possible for people to specify explicitly what indent amount they want in that code. Then, anyone editing that code will have their editor follow the style that the original author wanted.
CyrusNajmabadi commented 8 years ago

I don't think adding a switch for namespace and type would be effective. In example, I use the coding style described in this thread for all my projects, but I also have to edit Open Source libs which use the common C# indenting style.

I think the switch would help if we also have support for things like .editor_config. Then you can set this option locally for your own projects, without having it impact you when editing other libraries.

vik3000 commented 8 years ago

Hi, thanks for the response. I still think the original behavior was superior and more universal.

Switch solves just one specific case- what if someone else likes not to indent both namespace and first level of classes? (Maybe you could force him to change such habit, but what about code that already exists?)

Per file configuration sounds frustrating - would you want to fiddle with settings each time you open a file and add two lines to it?

And true, mixed indentation in one file is definitively bad practice, but it happens in real world. As a result of multiple devs with different indentation settings eventually editing the same file, for example. And once this happens and you have to maintain such a codebase, there is no way to do it in VS2015. Reformatting the code is not an option, mainly because it would fog the source control history.

rickpmartin commented 8 years ago

I couldn't agree more with vik3000, setting configuration per file would be so tedious that no indentation at all would still be preferable. As much as it would be nice to format all the "badly" formatted code, it would ruin the source control history. This situation is far from ideal, yet it is reality.

CyrusNajmabadi commented 8 years ago

Switch solves just one specific case- what if someone else likes not to indent both namespace and first level of classes?

It is an explicit non-goal of the formatting engine to support every "like" that every user has :smile:.

I couldn't agree more with vik3000, setting configuration per file would be so tedious

You wouldn't have to set it per file. .editor_config can work at a folder level as well. So if you have a project, or bunch of files that use this different format, you could specify that that was the case.

it would ruin the source control history.

I'm not understanding why source control history would be ruined if you normalized the formatting in your repo. We've done this ourselves in Roslyn on several occasions and it hasn't been an issue AFAICT.

And true, mixed indentation in one file is definitively bad practice, but it happens in real world.

I'm not a fan of adding support into our core engine for something which is considered bad practice. I'd rather the engine push people toward best practices (including using the formatting styles that are used by nearly the entire ecosystem of code out there).

As a result of multiple devs with different indentation settings eventually editing the same file, for example. And once this happens and you have to maintain such a codebase, there is no way to do it in VS2015. Reformatting the code is not an option, mainly because it would fog the source control history.

I'd like clarification on this. Why not just normalize the code and have a single commit that says "Make indentation uniform across the codebase". I'm not seeing how a single commit would really fog anything?

Indeed, i see tons of formatting related commits in the Roslyn code base. I'd honestly prefer to just it be one-and-done, rather than a continuous stream as people take errant code and bring it in line.

rickpmartin commented 8 years ago

The issue with source control is the "annotate" functionality, which shows when a line was modified and by whom. Reformatting an entire document changes every line, making annotate no longer useful.

KakCAT commented 8 years ago

It is an explicit non-goal of the formatting engine to support every "like" that every user has

at the root, it is not a matter of autoformatting. It is a matter of "style retrocompatibility". People has written their code as they feel it's better and has grown used to that (in my case, more than 2 1/2 decades) . VS2015 has broken that, and now users must choose between adapting to a new (arbitrarely defined by MS, no matter how right they are) style, keep a lot of code with "broken mixed formatting" or just ignore VS2015 (my choice so far).

I'm not a fan of adding support into our core engine for something which is considered bad practice. I'd rather the engine push people toward best practices (including using the formatting styles that are used by nearly the entire ecosystem of code out there).

I won't enter the debate of mixed spaces/tab or not, but the problems presented in most examples, specially the ones of @HaloFour , don't have mixed indenting but makes VS2015 a hell to work with.

Best definition of the problem and solution is @tunger's

Best would be like in VS2013, where the formatter/indentation would recognize the indent for the current code block, and apply the indentation size from there - rather than only multiplying indent size by the number of code blocks.

The fact is that this problem was well solved by previous versions, allowing users to format the code as they pleased, and now it's just broken for people not adhering exactly to the allowed formats.

No matter how many per file configuration files (which honestly I can't see what can fix regarding this issue because it's not a matter of files, but formatting) or option switches you add, there will be always somebody with a custom style complaining about the new formatting. And it's logical to complain, because it's been working well since VC6

CyrusNajmabadi commented 8 years ago

The fact is that this problem was well solved by previous versions,

This problem was. But other problems were not. And we decide what work to do based on the relative value we feel there is to the entire ecosystem. Other problems that we solved were ones we heard about from far more people and could see impacting far more codebases. Some of these changes did have unfortunate consequences for some styles of code. But that's the case whenever the formatter changes in any way. You always run the risk of someone saying "but i wanted it exactly how it worked before". The question then becomes: do you attempt to support both the new and old behavior, or do you deprecate certain functionality because the value isn't worth the cost. This is where that conversation is currently.

Personally, knowing how the formatting engine is designed today, i lean heavily against options. Each option (especially one as broad as 'how does indentation work') have a massive ripple effect with every thing in the formatting engine. This leads to an even greater testing/maintenance burden in the future, and opens up the ever increasing matrix size of things that might not work exactly right the moment we make any change anywhere in formatting.

I'm sympathetic to the argument that people get into a coding style that previous versions of hte product (intentionally or unintentionally) did not work against them with.

No matter how many per file configuration files (which honestly I can't see what can fix regarding this issue because it's not a matter of files, but formatting)

.editor_config files address the problem of "someone has different settings, comes to my code, and starts formatting in ways i don't want." They allow formatting settings to travel with teh code, versus traveling with the editor/user.

there will be always somebody with a custom style complaining about the new formatting.

Yes, you've hit the nail on the head. The question becomes then: is this custom style worth addressing? Personally, this is why i tend to avoid custom styles in all languages in all IDEs. :smile:

zvilius commented 7 years ago

We should probably add a "Do not indent namespace content" option.

This would help me.

Alternatively, I wouldn't mind using "Block Indenting," except that I also want to use "Automatically format statement on ;" and "Automatically format block on }" in order to apply my within-line (Spacing) formatting. If the "automatic formatting" options would respect my indenting when I'm not using "Smart Indenting" then I could accept that solution.

Personally, knowing how the formatting engine is designed today, i lean heavily against options.

Hmm, but isn't that a goal of any formatting engine: to provide at least some flexibility to accommodate individual preferences?

ghost commented 7 years ago

I have same issue, but more were added along the way:

  1. Yes I hate that useless 2*4 spaces for namespace + class and used the indentation as OP.
  2. The reformatter won't format every line, so, even if I press Ctrl+E,D to use the enforced style (to be able to edit it in VS2015), some (intra-expression) lines are left unchanged (and they are indented two tabs left relative to the surrounding lines, really ugly).
  3. I was also using "comma first" style in constructors or method calls/declarations (placing the comma at the start of the line instead of at the end). The reformatted result is full of extra new-lines.
  4. It keeps destroying my well indented enums (name tabs = 0xMASK tabs // comment)
  5. I want my preprocessor blocks indented like labels (one less than the block they are in), not having them all on the left side (imagine some deep level: namespace, class, method, switch, case, for, if .... if DEBUG - it gets so out of the real code that you won't even notice it is there)
  6. When some block is conditional (e.g. try..catch in DEBUG only), the formatter keeps reformatting (reindenting) it whenever I change the condition (switching from Debug to Release build).
  7. Conditionally disabled blocks (if DEBUG/RELEASE) are not formatted.
  8. lock(sync) for(;;) {... Indented twice (yes I do understand why, but that is so common and useless!) if(cond) do { ... } while(cond && cond2);, if(whatever) try { ... } catch ... etc.

Can I write my own formatter? I have already written VSIX with SingleFileGenerator (generates .cs files from my sources), so I know my way around a bit, but am no expert. Can somebody point me in right direction?

bbugh commented 7 years ago

I would really, really love IndentNamespaceContent. I'm using VSCode and the useless indentation for namespaces drives me crazy! 🦀

tmm360 commented 7 years ago

Any news on https://github.com/dotnet/roslyn/issues/4351#issuecomment-129926809? I'm having this issue on VS 2017 RTM.

JoelDKraft commented 7 years ago

This definitely does not appear to have been addressed in Visual Studio 2017.

My use case is slightly different that what you are discussing here, but I think it is the same underlying issue. I'm writing code inline in .aspx pages. There are two useless sets of indents. One is that it likes to indent every HTML tag in a page, and the second is that it wants to indent the contents of a Githubissues.

  • Githubissues is a development platform for aggregating issues.