Looking for feedback on some formatting changes


We’re considering making two changes to the way the formatting engine works, based on feedback we’ve received from Beta 1.  I’d like to post them here, and see what you all think of them, to make sure we’re making the right decision.


 


Change 1: Comments that start at column 1.


In Beta 1, if you have a comment that starts at column 1 (the very left edge of the screen), we won’t change it’s indentation.  The main reason for this behaviour is that if you select a block of text and use Edit->Advanced->Comment selection, we place all the ‘//’ that we generate at the left edge, and we wouldn’t want a subsequent formatting to push them way in.


 


We’re finding that this behaviour has bad effects in two main ways.



  1. It’s confusing.  It’s not really clear why we change the indentation of some comments, but not others.

  2. It has a negative effect on generated code.  Often it’s nice to be able to generate some code with comments in it(say with a snippet).  But it looks kind of ugly if either all you’re comments end up at the left margin, or you need to add spaces to the snippet file.

 


Our proposed solution to this is to make the formatting engine always indent comments, and fix our “Comment Selection” behaviour to generate comments at the indent of the line with the least indentation in a block.  For example comment selection would now generate something like this:



    public void Method()


    {


        List<int> list = new List<int>();


        li.Add(3);


        //foreach (int i in list)


        //{


        //    Console.WriteLine(i);


        //}


    }


Instead of:



    public void Method()


    {


        List<int> list = new List<int>();


        li.Add(3);


//      foreach (int i in list)


//      {


//          Console.WriteLine(i);


//      }


    }


 


Change 2: Anonymous methods default to having the open brace on the same line.


This change is mainly for readability.  Basically we want to change the default value of “Tools->Options->Text Editor->C#->Formatting->New Lines->New line options for braces->Place open brace on new line for anonymous methods” from true to false.  The reason for this is that we think that if you have code like this:


 



    List<Thing> things;


    void SomeMethod()


    {


        List<Thing> somethings = things.FindAll(delegate(Thing thing) {


            return thing.ShouldBeIncluded;


        });


    }


It’s easier to tell that it’s part of an anonymous method than:



        List<Thing> somethings = things.FindAll(delegate(Thing thing)


        { return thing.ShouldBeIncluded; }


        );


Or:



        List<Thing> somethings = things.FindAll(delegate(Thing thing)


        { return thing.ShouldBeIncluded; });


Or:



        List<Thing> somethings = things.FindAll(delegate(Thing thing)


        {


            return thing.ShouldBeIncluded;


        });


 


All of which look sort of like you just have a random block.


 


So what do you think?  Are we on the right track, or is there a better way to solve these problems?


 


Comments (21)

  1. Change #1:

    I’ve been meaning to say something about this. Not indenting the comment marker for commented out sections causes grief, since when the section gets re-formatted, the comments are indented and the original indentation is included in the comment.

    The way of placing the comment markers after the tabs is best.

    Change #2:

    I don’t like either one. How about:

    List<Thing> somethings = things.FindAll(

    _ _ delegate(Thing thing) {

    _ _ _ _ return thing.ShouldBeIncluded;

    _ _ });

    This depicts the anonymous method as close as possible to a normal method.

    If you have more parameters, you could do:

    List<Thing> somethings = things.FindAll(myParam,

    _ _ delegate(Thing thing) {

    _ _ _ _ return thing.ShouldBeIncluded;

    _ _ },

    _ _ myParam2, myParam3);

    Making anonymous methods ‘jump out of the code’ is essential, IMO.

  2. Ah. I just found a mistake in my comment… I meant for anonymous methods to be formatted like this:

    List<Thing> somethings = things.FindAll(myParam,

    _ _ delegate(Thing thing)

    _ _ {

    _ _ _ _ return thing.ShouldBeIncluded;

    _ _ },

    _ _ myParam2, myParam3);

    I don’t even like the ‘brace at the same line’ formatting… 🙂

  3. Personally I like

    List<Thing> somethings = things.FindAll

    (

    delegate(Thing thing)

    {

    return thing.ShouldBeIncluded;

    }

    );

    or

    List<Thing> somethings = things.FindAll(

    delegate(Thing thing)

    {

    return thing.ShouldBeIncluded;

    }

    );

    (I hope this formats well)

    I know it takes up a lot of vertical space, but it seems consistent with the way classes and methods are formatted by default, with the open brace always in the same column with the closing brace. In this case, I think using a non-standard open parens on its own line is justified because this is a new constuct (anonymous method) and deserving of a unique format.

  4. You’re example should work if you set the option to put the brace on the new line. We won’t remove the newlines that you add, or mess with the indentation of the other parameters if you specify it that way. If you try it in the express sku today, you should be able to format that way without any problems.

  5. Here’s a tip for putting code into a blog post: Type the code in VS. Copy and paste it into Word, then copy and paste it into the blog page.

  6. Really, you want VS to not change the formatting on comments made with the auto-comment function.

    So, why not add a globally useful feature: Have VS store formatting in a special comment at the end of the file. Then VS can act on more than just what the code looks like.

    Although, I’m guessing no one will like this, precisely because VS will be acting on more than the code looks…

    My vote would be to have the autoformating for all comments, regardless.

  7. Kevin,

    About the blog post thru MSWord: This doesn’t work in comments because HTML is reformatted to plain text.

    For instance: <b>not really bold text</b>.

  8. Jeff Davis says:

    I like your autoformatting idea for comments. I’ve always wished VS did something like this, since I also find the comments at the very left (or worse, ragged comment lines) to impair readability.

    I’d rather have braces on their own lines for anonymous methods (as well as anything else), but I might change my mind after I use anonymous methods.

  9. Jeff: You can still have the braces on their own lines for anonymous methods.

    All that’s changed is that now, by default, the open-curly will stay on the same line as the "delegate"

  10. Ben says:

    Change 1:

    I agree that the // should be immediately before the code that it comments out, instead of at column 1. In the current betas, I purposely avoid using the auto // feature because it does not place it where I want. Instead I do it manually. I would be most grateful for your proposed changed.

    Change 2:

    Lets label your examples from A (first) to D (last) for the sake of discussion. Personally, I never want braces on the same line, including anonymous methods. I am extremely opposed to choice A and will manually edit it everytime if need be should this become final. Next, B and C are also less than ideal for me. While slightly better than A, I want braces to be on their own away from the main code. I would, however, make an exception for choice D that places the final ); on the same line as the closing }.

    Summary: For change 1, comments next to code, not at column 1. For change 2, opening brace on a new line all by itself; the closing brace and ); together on a new line all by itself. Actual code to go in between these two lines.

  11. Ben, in your proposed solution for change 2, what would the indentation of the braces be, relative to the start of the line that contains the "delegate" keyword.

  12. Ben says:

    In my limited experience with anonymous methods in the beta so far, I prefer having the braces vertically lined up with the column that begins the line, not lined up with the ‘d’ in delegate. Thus, in your original example, I prefer to position the braces vertically in the same column as the ‘L’ in List.

    List<Thing> somethings = things.FindAll(delegate(Thing thing)

    {

    ____return thing.ShouldBeIncluded;

    });

    where _ is a space.

  13. Thanks, Ben. So what you are saying is that you like anonymous methods formatted the way they currently are, see my 4th example. We would definitely not be changing the ability to do that, just what the default value of the option is "Tools->Options" is.

    Based on this feedback however, it looks like we may want to leave the default alone.

  14. Phil Weber says:

    Stupid question from a VB guy: Why doesn’t C# use /* … */ for block comments? Wouldn’t that render this question moot?

  15. Phil, because then you can’t block comment anything that contains a /* */ style comment, because the new comment that you introduce would end at the first */

  16. Sascha says:

    Hi,

    for the block commenting: why don’t you use ‘// ‘ instead of ‘//’?

    //for(…) {

    // …

    //} // for

    is harder to read than

    // for(…) {

    // …

    // }

    Just my opinion.

    For the second change: I prefer opening braces on the same line everywhere, so I welcome any change towards this. Since I changed the VS.NET’s default templates to reflect this, this reduces my work. Basicly I think opening braces on a seperate line or not is a question of personal taste.

  17. Ron says:

    Kevin,

    Regarding Change 2, I was agree with most of the others and format it like you have in example D. IOW, keep the default as currently defined.

  18. Gavin Morris says:

    I’m all for Omers first post (I use braces on the same line as the construct). The same line/next line setting should be determined from the existing option.

  19. John Rusk says:

    If the underlying requirement is to show anonymous methods clearly, would it be feasible to actually colour code them? E.g. a subtle background shading for anonymous blocks?