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

No way to place long boolean expressions on separate lines #140

Open
fsareshwala opened this issue May 2, 2024 · 1 comment
Open

No way to place long boolean expressions on separate lines #140

fsareshwala opened this issue May 2, 2024 · 1 comment
Labels
enhancement New feature or request language feature

Comments

@fsareshwala
Copy link
Contributor

Suppose I have the following emboss definitions, along with a very long boolean expression:

enum PacketType:
  FOO = 0
  BAR = 1
  BAZ = 2
  ABC = 3
  DEF = 4
  GHI = 5

struct MyStruct:
  0 [+1] PacketType packet_type

  if packet_type == PacketType.FOO || packet_type == PacketType.BAR || packet_type == PacketType.BAZ || packet_type == PacketType.ABC|| packet_type == PacketType.DEF || packet_type == PacketType.GHI:
    # ...

For readability's sake, I'd like to be able to break up the boolean expression into multiple lines, like this:

struct MyStruct:
  0 [+1] PacketType packet_type

  if packet_type == PacketType.FOO ||
     packet_type == PacketType.BAR ||
     packet_type == PacketType.BAZ ||
     packet_type == PacketType.ABC||
     packet_type == PacketType.DEF ||
     packet_type == PacketType.GHI:
    # ...

Doing so results in the following error:

error: Syntax error
note: Found '\n' ("\n"), expected "$max", CamelWord, "(", SnakeWord, "$max_size_in_bits", "$upper_bound", "$is_statically_sized", "-", ShoutyWord, "$size_in_bytes", "$max_size_in_bytes", BooleanConstant, "$min_size_in_bytes", "$static_size_in_bits", "$present", "+", Number, "$lower_bound", "$next", "$size_in_bits", "$min_size_in_bits".
  if packet_type == PacketType.FOO ||
                                     ^

Placing the || as a line prefix doesn't work either:

struct MyStruct:
  0     [+1]    PacketType  packet_type
  if packet_type == PacketType.FOO
    || packet_type == PacketType.BAR
    || packet_type == PacketType.BAZ
    || packet_type == PacketType.ABC
    || packet_type == PacketType.DEF:
    # ...

Doing so results in the following error:

error: Syntax error
note: Found '\n' ("\n"), expected "-", "?", "&&", "==", "<", "+", ">", "<=", ":", ">=", "*", "||".
  if packet_type == PacketType.FOO
                                  ^

Using a backslash to indicate a continuation of the line on the next also doesn't work:

struct MyStruct:
  0     [+1]    PacketType  packet_type
  if packet_type == PacketType.FOO || \
    packet_type == PacketType.BAR || \
    packet_type == PacketType.BAZ || \
    packet_type == PacketType.ABC || \
    packet_type == PacketType.DEF:
    $next [+1]  UInt        data

Doing so results in the following error:

error: Unrecognized token
  if packet_type == PacketType.FOO || \
                                      ^

Using a backslash to indicate a continuation of the line along with using the || as a line prefix also doesn't work:

struct MyStruct:
  0     [+1]    PacketType  packet_type
  if packet_type == PacketType.FOO \
    || packet_type == PacketType.BAR \
    || packet_type == PacketType.BAZ \
    || packet_type == PacketType.ABC \
    || packet_type == PacketType.DEF:
    $next [+1]  UInt        data

Doing so results in the following error:

error: Unrecognized token
  if packet_type == PacketType.FOO \
                                   ^

It seems like there is no way to have multi-line boolean expressions at the moment.

@jasongraffius jasongraffius added the enhancement New feature or request label Jul 22, 2024
@reventlov
Copy link
Collaborator

I've been thinking about this a bit.

I would probably want to implement something like Python does, where if the lexer sees a newline inside of () [] or {} pairs it suppresses the newline token that it would otherwise emit. (Python also accepts \ to continue lines, but it's generally discouraged.)

The tricky things I can think of are:

  1. I would want to implement some kind of forced indentation requirement, so you would still need to indent the wrapped line. I'm not sure what the exact rule should be: I'm thinking something like:
    • If the most recent ([{ character was at the end of the previous line, that creates a new indentation context and everything inside needs to be consistently indented at least 1 space compared to the previous indentation, like:

      if (
        packet_type == PacketType.FOO ||
        packet_type == PacketType.BAR ||
        ...
      ):
      
    • If the most recent ([{ character had more characters after, then everything else must be indented as far as the other characters inside, like:

      if (packet_type == PacketType.FOO ||
          packet_type == PacketType.BAR ||
          ...):
      
  2. It may be difficult to have good error messages in cases where someone forgets a closing )]} character.
  3. The current Emboss grammar actually includes comment tokens (for use in the autoformatter). This isn't too disruptive in the current grammar because comments can only occur at EOL, and there are only a few places where EOLs can occur. This change would allow comments pretty much anywhere, which would make the grammar significantly larger (or we would have to use a different strategy to handle comments).
  4. Speaking of the autoformatter, it would need to start making real decisions about where/how to break lines, and some of those decisions are not at all trivial (see Clang-Format).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request language feature
Projects
None yet
Development

No branches or pull requests

3 participants