Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Indentation issue using #else block. #148

Closed
jcs090218 opened this issue Jul 23, 2019 · 13 comments
Closed

Indentation issue using #else block. #148

jcs090218 opened this issue Jul 23, 2019 · 13 comments
Labels
CC Mode bug tree-sitter Solved by using csharp-tree-sitter-mode

Comments

@jcs090218
Copy link
Collaborator

jcs090218 commented Jul 23, 2019

As mentioned in #147 conversation. You would get the error when trying to indent on line with #else with if (true) statement the before due to csharp-mode's indentation engine and this might requires a complete rewrite for this package.

@josteink
Copy link
Collaborator

Quick summary/test-case:

// working

#if DEBUG
Console.WriteLine("Yay!");
#else
Debug.WriteLine("Nay!");
#endif


// not working

#if DEBUG
if (!success || this.debugMode)
#else
    if (!success) // <-- incorrectly indented
#endif

@theothornhill
Copy link
Collaborator

@josteink @jcs090218
I'm struggling a little with this issue. Why do we want the if without any consequent?
This works fine now in rework branch.

#if DEBUG
Console.WriteLine("Yay!");
#else
Debug.WriteLine("Nay!");
#endif


#if DEBUG
if (!success || this.debugMode)
    Console.WriteLine("Yay!");
#else
if (!success)
    Debug.WriteLine("Nay!");
#endif

@josteink
Copy link
Collaborator

josteink commented Sep 25, 2020

It's a pseudo "not" operator for preprocessor directives.

#if ! THING
  // only when not thing
#endif

I'm not sure if that's allowed, but surely #if #else is.

#if THING
#else
  // only when not thing
#endif

@theothornhill
Copy link
Collaborator

theothornhill commented Sep 25, 2020

Thanks.
So it is okay to have the if (!success) without anything below it? I'm thinking here:

#if THING
#else
    if (!success)
        // Nothing needed here?
#endif

Just haven't seen that before, is all

@josteink
Copy link
Collaborator

josteink commented Sep 25, 2020

Oh reading the full thing, I think a wider context might be needed, and I've probably misrepresented it as well 😨

Consider the following C#, which is perfectly valid:

#if DEBUG
if (true) // always run this branch in debug-builds!
#else
if (!success)
#endif
{
    // do some logging, or whatever
}

In previous csharp-mode implementation this would result in indentation like this:

#if DEBUG
if (true) // always run this branch in debug-builds!
#else
    if (!success) // <-- incorrectly indented as a substatement of the one above if-statement
#endif
{
    // do some logging, or whatever
}

This is really hard to get right, I think, and if I were you, I wouldn't really lose my mind over it. If this doesn't work properly, it's fair to consider this an edge-case as far as common C#-usage is concerned.

@theothornhill
Copy link
Collaborator

Btw, is this valid in C or C++? I noticed CC Mode does not support this edge case there.

@jcs090218
Copy link
Collaborator Author

Btw, is this valid in C or C++?

@theothornhill Is valid C and C++.

I noticed CC Mode does not support this edge case there.

They don't?.... I am surprised.

@theothornhill
Copy link
Collaborator

Btw, is this valid in C or C++?

@theothornhill Is valid C and C++.

I noticed CC Mode does not support this edge case there.

They don't?.... I am surprised.

Yeah... So this is definitively not a bug in csharp-mode, but should maybe be upstreamed as a bug there. My C-fu is weak, so it would be nice if someone else could confirm my suspicion. However, if it works in C++, then we could absolutely find where that is done and copy it.

@theothornhill theothornhill added the tree-sitter Solved by using csharp-tree-sitter-mode label Dec 20, 2020
@theothornhill
Copy link
Collaborator

On tree-sitter branch:

namespace F
{
    public class Bar
    {
#if DEBUG
        Console.WriteLine("Yay!");
#else
        Debug.WriteLine("Nay!");
#endif


#if DEBUG
        if (!success || this.debugMode)
#else
        if (!success)
#endif
        {
        
        }
    }
}```

@jcs090218
Copy link
Collaborator Author

jcs090218 commented Dec 20, 2020

Wow!!!! This is awesome!

Does this bug appears in the upstream? Maybe we should apply to Emacs itself? Just wondering! But this is definitely awesome! 👍

@theothornhill
Copy link
Collaborator

In due time it will, I’m sure. The indent part is not in Melpa yet, so I’m keeping it for the time being here. As for adding it to emacs, the blocker for that is tree-sitter itself.

@jcs090218 could you try to use the tree sitter branch for a while? I’d like some more input on how it works :)

@jcs090218
Copy link
Collaborator Author

@jcs090218 could you try to use the tree sitter branch for a while? I’d like some more input on how it works :)

Yeah, sure. I will report issue if I have encountered one! Thanks for your hard work! 😄 This is definitely big step to Emacs itself.

@josteink
Copy link
Collaborator

csharp-mode is now developed as part of core emacs.

If you still have any issues, file a bug-report with the GNU Emacs bug-tracker :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
CC Mode bug tree-sitter Solved by using csharp-tree-sitter-mode
Projects
None yet
Development

No branches or pull requests

3 participants