Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow new line(s) after }: #241

Open
DavHau opened this issue Mar 3, 2022 · 5 comments
Open

Allow new line(s) after }: #241

DavHau opened this issue Mar 3, 2022 · 5 comments
Labels
component | style Modifications to the formatting rules good first issue Good for newcomers help wanted Extra attention is needed roadmap | style config status | it is a good thing We agree it is good to implement this type | feature request New feature or request

Comments

@DavHau
Copy link

DavHau commented Mar 3, 2022

Here a (modified) example from nixpkgs containing a long argument list followed by a function:

{
  lib
, autoPatchelfHook
, alsa-lib
, stdenv
, libXScrnSaver
, makeWrapper
, fetchurl
, wrapGAppsHook
, glib
, glibc
, gtk3
, unzip
, atomEnv
, libuuid
, at-spi2-atk
, at-spi2-core
, libdrm
, mesa
, libxkbcommon
, libappindicator-gtk3
, libxshmfence
, libXdamage
, nss
}:

version: hashes: let
  name = "electron-${version}";

  meta = with lib; {
    description = "Cross platform desktop application shell";
    homepage = "https://github.com/electron/electron";
    license = licenses.mit;
    maintainers = with maintainers; [ travisbhartwell manveru prusnak ];
    platforms = [ "x86_64-darwin" "x86_64-linux" "i686-linux" "armv7l-linux" "aarch64-linux" ]
      ++ optionals (versionAtLeast version "11.0.0") [ "aarch64-darwin" ];
    knownVulnerabilities = optional (versionOlder version "12.0.0") "Electron version ${version} is EOL";
  };
in
  true

It is clearly visible that a function with two parameters follows the argument list.
alejandra does this:

{
  lib,
  autoPatchelfHook,
  alsa-lib,
  stdenv,
  libXScrnSaver,
  makeWrapper,
  fetchurl,
  wrapGAppsHook,
  glib,
  glibc,
  gtk3,
  unzip,
  atomEnv,
  libuuid,
  at-spi2-atk,
  at-spi2-core,
  libdrm,
  mesa,
  libxkbcommon,
  libappindicator-gtk3,
  libxshmfence,
  libXdamage,
  nss,
}: version: hashes: let
  name = "electron-${version}";

  meta = with lib; {
    description = "Cross platform desktop application shell";
    homepage = "https://github.com/electron/electron";
    license = licenses.mit;
    maintainers = with maintainers; [travisbhartwell manveru prusnak];
    platforms =
      ["x86_64-darwin" "x86_64-linux" "i686-linux" "armv7l-linux" "aarch64-linux"]
      ++ optionals (versionAtLeast version "11.0.0") ["aarch64-darwin"];
    knownVulnerabilities = optional (versionOlder version "12.0.0") "Electron version ${version} is EOL";
  };
in
  true

The fact that there follows a function after the args list becomes hidden a bit.

Possible fix:

Always allow new lines (or even empty lines) after argument lists.

Downsides:

There will be multiple different styles allowed, vs right now there is only one single allows style, which has it's benefits as well.

Conclusion:

I'm not sure.

@kamadorueda kamadorueda added component | style Modifications to the formatting rules good first issue Good for newcomers help wanted Extra attention is needed status | it is a good thing We agree it is good to implement this type | feature request New feature or request labels Mar 3, 2022
@kamadorueda
Copy link
Owner

We can have the best of both worlds: enforcing the newlines, which means there is still one single style being allowed

The end result would be something like:

{
  libXdamage,
  nss,
}:

{
  libXdamage,
  nss,
}:

version: hashes: 

{
  libXdamage,
  nss,
}:

Most times I've seen nested lambdas in practice is because they are logically separated concepts

@DavHau
Copy link
Author

DavHau commented Mar 4, 2022

We can have the best of both worlds: enforcing the newlines, which means there is still one single style being allowed

Should this be a universal rule, or just if followed by a function?

@kamadorueda
Copy link
Owner

When the implementation is not another function it would look like a waste of space, but on the other hand, if we do it only for functions it would look inconsistent to the detail-oriented eye. If we allow choosing, then we sacrifice consistency as well, but overall would be an improvement since the user knows the perfect meaning

Other facts:

  • How common is this function inside function in Nix? not much common I'd say, I've normally seen it on frameworks, or library functions. More common in the side of people who produce code for other people to use

  • How do other languages deal with this function inside function? They sometimes inject the newlines to help distinguishing, sometimes not, there doesn't seem to be a common rule

I'm happy with all alternatives proposed so far, even leaving it as-is

@adrian-gierakowski
Copy link

imho, the example form #241 (comment) should be formatted as:

{
  libXdamage,
  nss,
}:
{
  libXdamage,
  nss,
}:
version: 
hashes: 
{
  libXdamage,
  nss,
}:

this gives equal treatment to function arguments, regardless if they are attrset destructuring style, or simple identifiers

currently it's formatted as follows:

{
  libXdamage,
  nss,
}: {
  libXdamage,
  nss,
}: version: hashes: {
  libXdamage,
  nss,
}:

since simple identifier arguments are always collapsed into a single line (even if it means making the line very long):

@adrian-gierakowski
Copy link

adrian-gierakowski commented Aug 28, 2022

here are some examples of how I'd prefer the formatter to behave in regards to function arguments:

in the following examples, developer "chooses" the formatting style, and the formatted enforces consistency: if all arguments are on a single line, the formatter leaves them as they are (unless they violate max line length), but inserting at least one newline, forces all into separate lines (same as it works currently for lists and operator chains):

unchanged:

a: b: c: a + b + c

in 1:

a: 
b: c: a + b + c

out 1:

a: 
b: 
c: 
a + b + c

in 2:

{
  a, 
  x
}: 
b: c: (a + b + c) * x

out 2:

{
  a, 
  x,
}: 
b: 
c: 
(a + b + c) * x

Then you could allow newlines, which would make the formatter "reset" the arguent formatting style (consitency only enforce per newline separated block), so the following would remain unchanged:

{
  a, 
  x,
}: 

b: c: (a + b + c) * x
a: 
x: 

b: c: (a + b + c) * x
a: x: 

b: 
c:
(a + b + c) * x

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
component | style Modifications to the formatting rules good first issue Good for newcomers help wanted Extra attention is needed roadmap | style config status | it is a good thing We agree it is good to implement this type | feature request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants