tweag / topiary

https://topiary.tweag.io/
MIT License
575 stars 29 forks source link

Nickel: Incorrectly inserts a newline before `=` in a record field-value pair #680

Closed silverraven691 closed 9 months ago

silverraven691 commented 9 months ago

Describe the bug

To Reproduce I write this

{
  config
    | {
      labels | { _ : String } = { a = "a", b = "b" },
    }
    | default
    = {},
}

Turns into this after a nickel format

{
  config
    | {
      labels | { _ : String } = { a
          = "a", b
          = "b" },
    }
    | default
    = {},
}

Which then turns into this after a second nickel format

{
  config
    | {
      labels | { _ : String } = {
          a
          = "a",
          b
          = "b"
        },
    }
    | default
    = {},
}

Expected behavior I expect no changes to happen.

Environment

Additional context This is stable

{
  config
    | {
      labels | { _ : String } = { a = "a", b = "b" },
    }
    | default
    #
    = {},
}
Xophmeister commented 9 months ago

Thank you for reporting this. Indeed, I can reproduce this and Topiary actually fails with this input, because the idempotency constraint is flaunted (Nickel must disable this).

That record which gets expanded (and ultimately fails) is on a single line, so I would expect no changes to the original input. If it were to span multiple lines, I would expect something like:

{
  a = "a",
  b = "b"
}

...which is clearly not happening, either :disappointed:

I will look into this...

(cc @yannham)

Xophmeister commented 9 months ago

Minimal reproducer:

{
  config
    | { labels = { a = "a" } }
    = {}
}
Xophmeister commented 9 months ago

The erroneous line break that's being added between the record field and the = sign, in the annotation, is due to this rule:

(
  (#scope_id! "annotated_assignment")
  "=" @prepend_spaced_scoped_softline
)

Either this rule is too general, or the scope is too wide...