Skip to content

Conversation

llakala
Copy link

@llakala llakala commented Jul 10, 2025

The first commit has a detailed description, I recommend reading it. I'd love to improve it before merging, since it does regress in a few areas.

Quick question: the multiline string indentation wasn't working for me. I'm using treesitter through nvim, and so wasn't getting my highlighting through :syntax on, so it didn't detect the synIDAttr. We could just remove the wrapping if statement, and check for '' generically. However, that would technically cause some regressions if there was a literal '' within some double quotes. What do you think is the best fix here? I can maintain a local patch if necessary.

llakala added 2 commits July 10, 2025 15:16
This commit fixes double-indent problems. Previously, any code with a
bracket on the next line would be indented like this:

{
  foo =
    {
      # todo
    };
}

After this commit, it's now properly indented like this:

{
  foo =
  {
    # todo
  };
}

This does cause some slight regressions, specifically when an equals
sign starts an expression that should be indented over. For example,
this code:

{
  combinedList =
    list1
    ++ list2
    ++ list3;
}

Now regresses into this:

{
  combinedList =
  list1
  ++ list2
  ++ list3;
}

To prevent these regressions, a multiline regex should really be used,
which would only increase indentation on an equals sign if:

1. the current line doesn't contain a semicolon
2. the next line didn't contain a starting bracket.

This seems quite simple in theory, I just wasn't able to get it working
myself. Some help on implementing this would be great if we'd prefer to
fix these regressions before merging.
@Ma27
Copy link
Collaborator

Ma27 commented Jul 10, 2025

After this commit, it's now properly indented like this:

{
 foo =
 {
   # todo
 };
}

Why is this being called "properly indented"?

Not only can't I recall a single instance of this being used, this also doesn't seem to be in line with what nixfmt does, i.e.

{
  foo =
    mapAttrs (x: y: x) {
      # ...
    };
}

Not only do I think, the current state is reasonable, this is also what the only widespread formatter does and I don't think an editor should work against this.

👎 from me.

@llakala
Copy link
Author

llakala commented Jul 11, 2025

Sorry, I think I gave some bad examples.

What I should've explained better is that the current behavior results in misaligned indentation. For example, this code gets misaligned by a level:

{
  foo =
    {
      bar = 3;
    };
  }

And the code you shared actually gets misaligned too:

{
  foo =
    mapAttrs (x: y: x) {
      # ...
    };
  }

The reason I mentioned using a better multiline regex instead is because I think we SHOULD be indenting over by a level for something like the code above (ideally without misaligning the rest of the file). I just felt that when we have a bracket on the next line, the double-indent didn't make as much sense. I'd like to prevent the double indent without breaking functions that are indented over by a level: I'm just not very good at writing Vimscript.

Sidenote: nixfmt-rfc-style doesn't actually format that code like that. It reformats it to this:

{
  foo = mapAttrs (x: y: x) {
    # ...
  };
}

@Ma27
Copy link
Collaborator

Ma27 commented Jul 12, 2025

Sidenote: nixfmt-rfc-style doesn't actually format that code like that. It reformats it to this:

Apologies, my point wasn't clear enough. The indent does indeed not happen in my example, but only if the attribute-name is pretty long. Another example of the "indent after assignment operator" is

{
  foo =
    let
      xyz = 2;
      abc.def = xyz;
    in
    {
      inherit xyz abc;
      inherit (abc) def;
    };

}

What I should've explained better is that the current behavior results in misaligned indentation. For example, this code gets misaligned by a level:

OK, I see the problem now.

I do wonder however, is there a non-shitty way to make it dedent by two levels of indent in the case you cited?

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.

2 participants