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

Don't expand lists within functions #233

Merged
merged 2 commits into from
Sep 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions src/Nixfmt/Pretty.hs
Original file line number Diff line number Diff line change
Expand Up @@ -375,15 +375,34 @@ instance Pretty Parameter where
-- then start on a new line instead".
prettyApp :: Bool -> Doc -> Bool -> Expression -> Expression -> Doc
prettyApp indentFunction pre hasPost f a =
let -- This is very hacky, but selections shouldn't be in a priority group,
let -- Walk the function call chain
absorbApp :: Expression -> Doc
-- This is very hacky, but selections shouldn't be in a priority group,
-- because if they get expanded before anything else,
-- only the `.`-and-after part gets to a new line, which looks very odd
absorbApp (Application f' a'@(Term Selection{})) = group' Transparent (absorbApp f') <> line <> nest (group' RegularG a')
absorbApp (Application f' a') = group' Transparent (absorbApp f') <> line <> nest (group' Priority a')
absorbApp (Application f' a'@(Term Selection{})) = group' Transparent (absorbApp f') <> line <> nest (group' RegularG $ absorbInner a')
absorbApp (Application f' a') = group' Transparent (absorbApp f') <> line <> nest (group' Priority $ absorbInner a')
-- First argument
absorbApp expr
| indentFunction && null comment' = nest $ group' RegularG $ line' <> pretty expr
| otherwise = pretty expr

-- Render the inner arguments of a function call
absorbInner :: Expression -> Doc
-- If lists have only simple items, try to render them single-line instead of expanding
-- This is just a copy of the list rendering code, but with `sepBy line` instead of `sepBy hardline`
absorbInner (Term (List paropen@Ann{trailComment = post'} items parclose))
| length (unItems items) <= 4 && all (isSimple . Term) items =
pretty (paropen{trailComment = Nothing})
<> surroundWith sur (nest $ pretty post' <> sepBy line (unItems items))
<> pretty parclose
where
-- If the brackets are on different lines, keep them like that
sur = if sourceLine paropen /= sourceLine parclose then hardline else line
absorbInner expr = pretty expr

-- Render the last argument of a function call
absorbLast :: Expression -> Doc
absorbLast (Term t)
| isAbsorbable t =
group' Priority $ nest $ prettyTerm t
Expand Down Expand Up @@ -522,7 +541,11 @@ absorbExpr _ expr = pretty expr
-- Render the RHS value of an assignment or function parameter default value
absorbRHS :: Expression -> Doc
absorbRHS expr = case expr of
-- Absorbable expression. Always start on the same line
-- Exception to the case below: Don't force-expand attrsets if they only contain a single inherit statement
(Term (Set _ _ binders _))
| case unItems binders of [Item (Inherit{})] -> True; _ -> False ->
hardspace <> group (absorbExpr False expr)
-- Absorbable expression. Always start on the same line, and force-expand attrsets
_ | isAbsorbableExpr expr -> hardspace <> group (absorbExpr True expr)
-- Parenthesized expression. Same thing as the special case for parenthesized last argument in function calls.
(Term (Parenthesized open expr' close)) -> hardspace <> absorbParen open expr' close
Expand Down
2 changes: 1 addition & 1 deletion src/Nixfmt/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ data Item a
Comments Trivia
deriving (Foldable, Show, Functor)

newtype Items a = Items {unItems :: [Item a]} deriving (Functor)
newtype Items a = Items {unItems :: [Item a]} deriving (Functor, Foldable)

instance (Eq a) => Eq (Items a) where
(==) = (==) `on` concatMap Data.Foldable.toList . unItems
Expand Down
2 changes: 1 addition & 1 deletion test/diff/apply/in.nix
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
mapAttrsToStringsSep "\n" mkSection attrsOfAttrs;
}
[
(mapAttrsToStringsSep [force long] "\n" mkSection attrsOfAttrs)
(mapAttrsToStringsSep [force /* meow */ long] "\n" mkSection attrsOfAttrs)
]
(a
b)
Expand Down
16 changes: 5 additions & 11 deletions test/diff/apply/out-pure.nix
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
}
[
(mapAttrsToStringsSep [
force
force # meow
long
] "\n" mkSection attrsOfAttrs)
]
Expand Down Expand Up @@ -160,16 +160,10 @@
''"''
"\${"
];
escapeMultiline =
libStr.replaceStrings
[
"\${"
"''"
]
[
"''\${"
"'''"
];
escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [
"''\${"
"'''"
];
test =
foo
[
Expand Down
2 changes: 1 addition & 1 deletion test/diff/apply/out.nix
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
}
[
(mapAttrsToStringsSep [
force
force # meow
long
] "\n" mkSection attrsOfAttrs)
]
Expand Down
30 changes: 6 additions & 24 deletions test/diff/idioms_lib_3/out-pure.nix
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,7 @@ rec {
toINI =
{
# apply transformations (e.g. escapes) to section names
mkSectionName ? (
name:
libStr.escape [
"["
"]"
] name
),
mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
# format a setting line from key and value
mkKeyValue ? mkKeyValueDefault { } "=",
# allow lists as values for duplicate keys
Expand Down Expand Up @@ -191,13 +185,7 @@ rec {
toINIWithGlobalSection =
{
# apply transformations (e.g. escapes) to section names
mkSectionName ? (
name:
libStr.escape [
"["
"]"
] name
),
mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
# format a setting line from key and value
mkKeyValue ? mkKeyValueDefault { } "=",
# allow lists as values for duplicate keys
Expand Down Expand Up @@ -378,16 +366,10 @@ rec {
''"''
"\${"
];
escapeMultiline =
libStr.replaceStrings
[
"\${"
"''"
]
[
"''\${"
"'''"
];
escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [
"''\${"
"'''"
];
singlelineResult =
''"'' + concatStringsSep "\\n" (map escapeSingleline lines) + ''"'';
multilineResult =
Expand Down
30 changes: 6 additions & 24 deletions test/diff/idioms_lib_3/out.nix
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,7 @@ rec {
toINI =
{
# apply transformations (e.g. escapes) to section names
mkSectionName ? (
name:
libStr.escape [
"["
"]"
] name
),
mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
# format a setting line from key and value
mkKeyValue ? mkKeyValueDefault { } "=",
# allow lists as values for duplicate keys
Expand Down Expand Up @@ -194,13 +188,7 @@ rec {
toINIWithGlobalSection =
{
# apply transformations (e.g. escapes) to section names
mkSectionName ? (
name:
libStr.escape [
"["
"]"
] name
),
mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
# format a setting line from key and value
mkKeyValue ? mkKeyValueDefault { } "=",
# allow lists as values for duplicate keys
Expand Down Expand Up @@ -391,16 +379,10 @@ rec {
''"''
"\${"
];
escapeMultiline =
libStr.replaceStrings
[
"\${"
"''"
]
[
"''\${"
"'''"
];
escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [
"''\${"
"'''"
];
singlelineResult =
''"'' + concatStringsSep "\\n" (map escapeSingleline lines) + ''"'';
multilineResult =
Expand Down
24 changes: 6 additions & 18 deletions test/diff/idioms_lib_4/out-pure.nix
Original file line number Diff line number Diff line change
Expand Up @@ -521,33 +521,25 @@ rec {
# the normalized name for macOS.
macos = {
execFormat = macho;
families = {
inherit darwin;
};
families = { inherit darwin; };
name = "darwin";
};
ios = {
execFormat = macho;
families = {
inherit darwin;
};
families = { inherit darwin; };
};
# A tricky thing about FreeBSD is that there is no stable ABI across
# versions. That means that putting in the version as part of the
# config string is paramount.
freebsd12 = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
name = "freebsd";
version = 12;
};
freebsd13 = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
name = "freebsd";
version = 13;
};
Expand All @@ -557,19 +549,15 @@ rec {
};
netbsd = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
};
none = {
execFormat = unknown;
families = { };
};
openbsd = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
};
solaris = {
execFormat = elf;
Expand Down
24 changes: 6 additions & 18 deletions test/diff/idioms_lib_4/out.nix
Original file line number Diff line number Diff line change
Expand Up @@ -521,33 +521,25 @@ rec {
# the normalized name for macOS.
macos = {
execFormat = macho;
families = {
inherit darwin;
};
families = { inherit darwin; };
name = "darwin";
};
ios = {
execFormat = macho;
families = {
inherit darwin;
};
families = { inherit darwin; };
};
# A tricky thing about FreeBSD is that there is no stable ABI across
# versions. That means that putting in the version as part of the
# config string is paramount.
freebsd12 = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
name = "freebsd";
version = 12;
};
freebsd13 = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
name = "freebsd";
version = 13;
};
Expand All @@ -557,19 +549,15 @@ rec {
};
netbsd = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
};
none = {
execFormat = unknown;
families = { };
};
openbsd = {
execFormat = elf;
families = {
inherit bsd;
};
families = { inherit bsd; };
};
solaris = {
execFormat = elf;
Expand Down
4 changes: 1 addition & 3 deletions test/diff/idioms_nixos_1/out-pure.nix
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,7 @@ in
})

(mkIf (!config.boot.isContainer) {
system.build = {
inherit kernel;
};
system.build = { inherit kernel; };

system.modulesTree = [ kernel ] ++ config.boot.extraModulePackages;

Expand Down
4 changes: 1 addition & 3 deletions test/diff/idioms_nixos_1/out.nix
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,7 @@ in
})

(mkIf (!config.boot.isContainer) {
system.build = {
inherit kernel;
};
system.build = { inherit kernel; };

system.modulesTree = [ kernel ] ++ config.boot.extraModulePackages;

Expand Down
Loading