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

Indentation inside lambda is wrong #105

Closed
jojojames opened this issue Feb 15, 2017 · 8 comments
Closed

Indentation inside lambda is wrong #105

jojojames opened this issue Feb 15, 2017 · 8 comments

Comments

@jojojames
Copy link
Contributor

Using c-show-syntactic-information.

The syntactic information does not seem to be consistent so it's hard to write the indentation settings for it.

[Test(Description = "BLAH")]
public void TestFunction()
{
    AsyncTest.Run(async () =>
    /*|*/
    { // inexpr-class , brace-list-open
        /*|*/RandomObject abc = // brace-list-intro
        /*|*/new RandomObject(); // brace-list-entry
        // ^ Not lined up
    });
}

// WORKS
[Test(Description = "BLAH")]
public void TestFunction2()
{ // defun-open
    RandomObject obj = // defun-block-intro
        new RandomObject(); // statement-cont[3]
}

My settings, although I've also tested with the default c style from csharp mode.

(c-add-style
 "csharp"
 '("Java"
   (c-basic-offset . 4)
   (c-comment-only-line-offset 0 . 0)
   (c-offsets-alist
    (access-label . -)
    (arglist-close . c-lineup-arglist)
    (arglist-cont . 0)
    (arglist-cont-nonempty . c-lineup-arglist)
    (arglist-intro  . +)
    (block-close . 0)
    (block-open . 0)
    (brace-entry-open . 0)
    (brace-list-close . 0)
    (brace-list-entry . 0)
    (brace-list-intro . +)
    (brace-list-open . 0)
    (c . c-lineup-C-comments)
    (case-label . +)
    (catch-clause . 0)
    (class-close . 0)
    (class-open . 0)
    (comment-intro . c-lineup-comment)
    (cpp-macro . 0)
    (cpp-macro-cont . c-lineup-dont-change)
    (defun-block-intro . +)
    (defun-close . 0)
    (defun-open . 0)
    (do-while-closure . 0)
    (else-clause . 0)
    (extern-lang-close . 0)
    (extern-lang-open . 0)
    (friend . 0)
    (func-decl-cont . 0)
    (inclass . +)
    (inexpr-class . 0)
    (inexpr-statement . 0)
    (inextern-lang . +)
    (inher-cont . c-lineup-multi-inher)
    (inher-intro . +)
    (inlambda . c-lineup-inexpr-block)
    (inline-close . 0)
    (inline-open . 0)
    (innamespace . +)
    (knr-argdecl . 0)
    (knr-argdecl-intro . 5)
    (label . 0)
    (lambda-intro-cont . +)
    (member-init-cont . c-lineup-multi-inher)
    (member-init-intro . +)
    (namespace-close . 0)
    (namespace-open . 0)
    (statement . 0)
    (statement-block-intro . +)
    (statement-case-intro . +)
    (statement-case-open . +)
    (statement-cont . +)
    (stream-op . c-lineup-streamop)
    (string . c-lineup-dont-change)
    (substatement . 0)
    (substatement-label . +)
    (substatement-open . 0)
    (template-args-cont c-lineup-template-args +)
    (topmost-intro . 0)
    (topmost-intro-cont . 0))))
@josteink
Copy link
Collaborator

Thanks for the report!

Do you have this issue without the async keyword, or does it show up regardless?

I quite recently made some changes with regard to how indentation works in order to solve another bug.

It would be interesting to know if this error is a result of that or not.

Is this something you know used to work? If not, would you be willing to test on an earlier commit than that? If you install from MELPA STABLE, the version there (0.9.0) should not contain this latest fix and may be a good candidate to test against.

@josteink josteink added the bug label Feb 16, 2017
@jojojames
Copy link
Contributor Author

jojojames commented Feb 19, 2017

It was about the same.

Used the file here when testing also.

https://stable.melpa.org/packages/csharp-mode-0.9.0.el

[Test(Description = "BLAH")]
public void TestFunction()
{
    AsyncTest.Run(async () =>
                  /*|*/
    { // inexpr-class , brace-list-open
        /*|*/RandomObject abc = // brace-list-intro
        /*|*/new RandomObject(); // brace-list-entry
        // ^ Not lined up
    });
}

// WORKS
[Test(Description = "BLAH")]
public void TestFunction2()
{ // defun-open
    RandomObject obj = // defun-block-intro
        new RandomObject(); // statement-cont[3]
}


public void TestFunction3()
{
    AsyncTest.Run(() =>
    { // inexpr-class, brace-list-open
        RandomObject b = // brace-list-intro
        new RandomObject(); // brace-list-entry
    });
}

public void TestFunction3()
{
    Test.Run(() =>
    { // inexpr-class, brace-list-open
        RandomObject b = // brace-list-intro
        new RandomObject(); // brace-list-entry
    });
}

@theothornhill
Copy link
Collaborator

This is now fixed in the rework branch as well

@josteink
Copy link
Collaborator

When testing this in the rework branch, I get the following indentation:

[Test(Description = "BLAH")]
public void TestFunction()
{
    AsyncTest.Run(async () =>
                  /*|*/
                  { // inexpr-class , brace-list-open
                      /*|*/RandomObject abc = // brace-list-intro
                      /*|*/new RandomObject(); // brace-list-entry
                      // ^ Not lined up
                  });
}

// WORKS
[Test(Description = "BLAH")]
public void TestFunction2()
{ // defun-open
    RandomObject obj = // defun-block-intro
        new RandomObject(); // statement-cont[3]
}


public void TestFunction3()
{
    AsyncTest.Run(() =>
    { // inexpr-class, brace-list-open
        RandomObject b = // brace-list-intro
            new RandomObject(); // brace-list-entry
    });
}

public void TestFunction3()
{
    Test.Run(() =>
    { // inexpr-class, brace-list-open
        RandomObject b = // brace-list-intro
            new RandomObject(); // brace-list-entry
    });
}

For me, that is mostly exactly like I'd want it to be, with one (1) exception 😀

The first case with async in it seems to trigger a different indentation code-path? But in VSCode and Visual Studio this block is indented like a normal lambda:

[Test(Description = "BLAH")]
public void TestFunction()
{
    AsyncTest.Run(async () =>
    /*|*/
    { // inexpr-class , brace-list-open
        /*|*/RandomObject abc = // brace-list-intro
        /*|*/new RandomObject(); // brace-list-entry
        // ^ Not lined up
    });

//  /\
//  L____ Indentation based on AsyncTest.Run-call.
}			

If it's possible to make csharp-mode indent like that, that would be great. We're like 99.9% there now 😮

@theothornhill
Copy link
Collaborator

If you remove the comment

    AsyncTest.Run(async () =>
                  /*|*/                                                 // <------------------- this one
                  { // inexpr-class , brace-list-open
                      /*|*/RandomObject abc = // brace-list-intro
                      /*|*/new RandomObject(); // brace-list-entry
                      // ^ Not lined up
                  });

It works!

@josteink
Copy link
Collaborator

I'm speechless. That's amazing!

At what point do we replace master with rework then? As in actually shipping it 🚢 📦 🚀

(Can you tell I'm excited about all this?)

@theothornhill
Copy link
Collaborator

theothornhill commented Sep 28, 2020

Yay! I think it is getting ready!

It is missing the imenu stuff - should we put that back in? Is that something that could be fsf compliant?
I can open an issue on this

(Can you tell I'm excited about all this?)

That's great to hear :)

@josteink
Copy link
Collaborator

It is missing the imenu stuff - should we put that back in?

I guess we could... But I'm not really using csharp-mode much these days for "real" editing without either lsp-mode or omnisharp-mode.

Is that something that could be fsf compliant?

Yes. I've scanned the current code for commits and contributors. 99.99% of the code there is written by me, and I have assigned copyright with FSF.

There's 3-4 external contributors, fixing mostly documentation (and one instance of configuration). Nothing we can't do from scratch if we need to. And most importantly: Nothing of Dino's original code remains, so his copyright does not need to be considered.

This should be very simple to get past the FSF legal-gate if we want to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants