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

Styling compiler specs uniformly #11291

Open
HertzDevil opened this issue Oct 7, 2021 · 5 comments
Open

Styling compiler specs uniformly #11291

HertzDevil opened this issue Oct 7, 2021 · 5 comments

Comments

@HertzDevil
Copy link
Contributor

#11269 raised some questions about how compiler specs should be styled. As mentioned there, there is some motivation to at least replace code literals with heredocs because they support better error locations. Due to the sheer number of compiler specs, any attempt to unify their style would require a massive effort to implement or undo, so we should agree upon a common style before doing so. My preferences are:

  • All multiline code literals should be enclosed in <<-CR heredocs. The contents should be indented one level relative to the assertion call:
    assert_type(<<-CR) { int32 }
      # code
      CR
    
    run(<<-CR).to_i.should eq(1)
      # code
      CR
  • Any remaining arguments go in the same line. The block argument also goes in the same line unless it is really long, in which case the preceding closing parenthesis starts a new line:
    assert_type(<<-CR, inject_primitives: true
      # code
      CR
    ) do
      # really long block
    end
  • All multiline error messages should be enclosed in <<-ERROR heredocs. The heredocs themselves should be indented one level relative to the assertion call, and placed entirely after <<-CR or any other preceding heredocs: (this is mainly because most Crystal syntax highlighters don't play well with multiple heredocs created on the same line)
    assert_error <<-CR,
      Foo
      CR
      <<-ERROR
      Undefined
      constant 'Foo'
      ERROR
  • If there are multiple heredocs like above, any remaining arguments would also start new lines (otherwise they would immediately follow <<-ERROR which disrupts the flow of those literals):
    assert_error <<-CR,
      CR
      <<-ERROR,
      ERROR
      flags: "foo"
  • Parentheses for the assertion calls are omitted unless a block is required (e.g. assert_type).
  • The contents of <<-CR literals should themselves be properly formatted Crystal code. There is no way to enforce this yet; it would be a good idea for the formatter tool to support this.
  • Single-line code literals can stay if they have only a single expression. Anything else should use <<-CR.
@straight-shoota
Copy link
Member

I'm not too happy about the handling of multiple heredocs. I would prefer to place the starting delimiters in the same line where feasible. That keeps the call context closely together.

assert_error <<-CR, <<-ERROR, flags: "foo"
  CR
  ERROR

If editors have problems with that, let's fix the syntax highlighting instead of working around its issues.

The contents of <<-CR literals should themselves be properly formatted Crystal code. There is no way to enforce this yet; it would be a good idea for the formatter tool to support this.

Yes, I think this is another benefit of heredocs. It gives context to the literal. We can use that to apply the formatter to embedded code and for syntax highlighting.

Single-line code literals can stay if they have only a single expression. Anything else should use <<-CR.

I don't think this needs to be a strict rule. Short mutli-line code in double quoted string literals can be okay for some use cases. Many parser and formatter specs are probably better readable as single-line string literals with escaped linebreaks.

@HertzDevil
Copy link
Contributor Author

To be precise the syntax highlighters I'm referring to are the Visual Studio Code one and the Atom one (and by extension, the one on GitHub itself). It appears that the same heredocs are handled properly for Ruby in these places.

@yxhuvud
Copy link
Contributor

yxhuvud commented Oct 8, 2021

And this is where I note that crystal lack <<~ (added in Ruby 2.3). Or in other words, I'd prefer indentation like

assert_error <<~CR, <<~ERROR, flags: "foo"
  foo
CR
  bar
ERROR

Letting the contents be indented relative to the end marker makes it so much easier to read. Though that is perhaps a separate discussion..

BTW, I'd definitely not assume heredocs to be intended to be formatted as crystal. By default they should be treated as strings and left alone. If some editors don't recognize them properly then that is a bug in those editors. Perhaps with a special flag though?

@asterite
Copy link
Member

asterite commented Oct 8, 2021

You can already do that. Indentation doesn’t matter in code.

@yxhuvud
Copy link
Contributor

yxhuvud commented Oct 8, 2021

For most of these particular cases, that is probably true. Unless there are specs where the indentation actually do matter (like for example the specs for heredocs themselves, I assume). Using <<- while indenting the contents will indent the resulting strings after all.

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

4 participants