textmate / swift.tmbundle

TextMate support for Swift
72 stars 30 forks source link

Treat `#if false` as a code, not a block comment #41

Closed akkyie closed 1 year ago

akkyie commented 4 years ago

Hi maintainers,

Currently, the defined syntax treats the body inside #if false as a block comment, not code. But I'd say it's code, because the Swift compiler does parse it and compilation fails if it's syntactically incorrect. I confirmed this behavior with swiftc 3.1.1 and the latest with https://godbolt.org/z/TTNu_R.

As a pragmatic reason, I'm doing kind of the trunk-based development with my team where we tend to temporarily disable alpha features with #if false to merge them into master branch early. This change make reviewing them much easier on GitHub.

I'm happy to hear if there is any reason not to do this. Thanks!

jtbandes commented 4 years ago

I believe this was included based on precedent from the C/C++ language support:

https://github.com/textmate/c.tmbundle/blob/39898a8af42d2afa667ce5238e3765704d0d173e/Syntaxes/C.plist#L851

I would also lean towards saying it is a block comment because the code is unconditionally disabled...

Although I notice for some reason GitHub does not highlight this as a comment in c/c++:

#if 0
void main() {}
#else
int main() {}
#endif

@infininight @sorbits thoughts?

sorbits commented 4 years ago

I would also lean towards saying it is a block comment because the code is unconditionally disabled...

My immediate thought is the same: It’s styled as a comment because the reader (of the source) can ignore it in figuring out what the program does.

That the compiler may have rules about syntax needing to be well-formed is secondary, and a linter could point out any syntax errors in the #if false block, even if it’s styled as a comment.

akkyie commented 4 years ago

@jtbandes @sorbits Thank you for comments! I think it's fair that it's not highlighted in #if false of C/C++ because you can write literally anything in it, while not in Swift.

It’s styled as a comment because the reader (of the source) can ignore it in figuring out what the program does.

Hmm. It'd sound better to me if, for example, a #if false-ed code block gets slightly darkened while keeping it's colors (I don't know whether it should be done by this syntax definition file or as a editor/tool feature,) but being not styled completely would rather make it harder to read it to figure out what happens if once it's enabled IMHO. I'm also wondering if this argument can be applied for the normal if false (which is almost always eliminated by the compiler too AFAIK.)

That the compiler may have rules about syntax needing to be well-formed is secondary, and a linter could point out any syntax errors in the #if false block, even if it’s styled as a comment.

This is not a strong opinion because it's not my primary motivation, but isn't it rather confusing for editor users if they see compiler/linter warnings on what looks like comments? 🤔

jtbandes commented 1 year ago

Closing since I've moved maintenance of the grammar over to https://github.com/jtbandes/swift-tmlanguage. Would be open to revisiting this concern if we can establish there's a "best practice" for #if false-like constructs in other popular grammars that will work well in VS Code & GitHub.