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

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

Closed
silverraven691 opened this issue Feb 16, 2024 · 3 comments · Fixed by #682
Closed

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

silverraven691 opened this issue Feb 16, 2024 · 3 comments · Fixed by #682
Assignees
Labels
language: nickel Nickel formatting issues P2 major: an upcoming release type: bug

Comments

@silverraven691
Copy link

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

  • Version of the code: Nickel v1.4.1

Additional context
This is stable

{
  config
    | {
      labels | { _ : String } = { a = "a", b = "b" },
    }
    | default
    #
    = {},
}
@Xophmeister
Copy link
Member

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 😞

I will look into this...

(cc @yannham)

@Xophmeister Xophmeister self-assigned this Feb 19, 2024
@Xophmeister Xophmeister added P2 major: an upcoming release type: bug language: nickel Nickel formatting issues labels Feb 19, 2024
@Xophmeister
Copy link
Member

Xophmeister commented Feb 19, 2024

Minimal reproducer:

{
  config
    | { labels = { a = "a" } }
    = {}
}

@Xophmeister
Copy link
Member

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...

Xophmeister added a commit that referenced this issue Feb 20, 2024
Previously, a line break was prepended to _any_ `=` sign within an
`annotated_assignment` scope. This was too liberal, if `=` signs
appeared within annotations themselves.

Resolves #680
Xophmeister added a commit that referenced this issue Feb 21, 2024
* Nickel: Line break only before binding = in annotations

Previously, a line break was prepended to _any_ `=` sign within an
`annotated_assignment` scope. This was too liberal, if `=` signs
appeared within annotations themselves.

Resolves #680

* Nickel: Remove `annotated_assigment` scope as now redundant
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language: nickel Nickel formatting issues P2 major: an upcoming release type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants