Skip to content

fix(code-action): preserve non-matching bindings in bulk Pack All#845

Open
inclyc wants to merge 1 commit into
mainfrom
fix/pack-all-non-contiguous
Open

fix(code-action): preserve non-matching bindings in bulk Pack All#845
inclyc wants to merge 1 commit into
mainfrom
fix/pack-all-non-contiguous

Conversation

@inclyc

@inclyc inclyc commented Jun 25, 2026

Copy link
Copy Markdown
Member

The bulk "Pack all 'X' bindings to nested set" and "Recursively pack all ..." actions computed a single contiguous range from the leftmost to the rightmost matching sibling and replaced it with the packed text. When matching siblings were non-contiguous, that range engulfed intermediate non-matching bindings, which the replacement silently dropped.

For example, { a.x = 1; b.y = 2; a.z = 3; } produced { a = { x = 1; z = 3; }; } instead of preserving b.y = 2;.

Emit a multi-edit WorkspaceEdit instead: replace the first matching binding with the packed text, then delete each subsequent matching binding in place. Intermediate non-matching bindings are left untouched.

Tighten the existing non-contiguous and shallow-bulk lit tests to verify the two-edit shape, and add a regression test that mirrors the original bug report.

Fixes: #837

The bulk "Pack all 'X' bindings to nested set" and "Recursively pack all
..." actions computed a single contiguous range from the leftmost to the
rightmost matching sibling and replaced it with the packed text. When
matching siblings were non-contiguous, that range engulfed intermediate
non-matching bindings, which the replacement silently dropped.

For example, `{ a.x = 1; b.y = 2; a.z = 3; }` produced
`{ a = { x = 1; z = 3; }; }` instead of preserving `b.y = 2;`.

Emit a multi-edit WorkspaceEdit instead: replace the first matching
binding with the packed text, then delete each subsequent matching
binding in place. Intermediate non-matching bindings are left untouched.

Tighten the existing non-contiguous and shallow-bulk lit tests to
verify the two-edit shape, and add a regression test that mirrors the
original bug report.
@MrQubo

MrQubo commented Jun 25, 2026

Copy link
Copy Markdown

Could we make it so it doesn't leave empty lines?
E.g.

{
  a.x = 1;
  b = 2;
  a.y = {
    d = 3;
  };
  c = 4;
}

using code actions gives:

{
  a = { x = 1; y = {
    d = 3;
  }; };
  b = 2;
  
  c = 4;
}

but I think it would be better to produce:

{
  a = { x = 1; y = {
    d = 3;
  }; };
  b = 2;
  c = 4;
}

It shouldn't remove empty lines that were already present, so e.g.

{
  a.x = 1;
  a.y = 3;

}

should be transformed to:

{
  a = { x = 1; y = 3; };

}

Implementation wise I think, the deletion should extend to the beginning and to the end of the line (including end of the line), as long as it's only whitespace.Z

I'm asking for this, because nixfmt won't remove single empty lines, as it leaves empty lines on purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Pack all" removes unrelated attributes

2 participants