Formatting issues and changes between 1 and 2 #880
Comments
|
Some changes seem like new quirks, though they don’t actually affect semantics, like - (!tree.constant) || tree.val == constant_list[node_index.val::UInt16]
+ (!tree.constant) || tree.val == constant_list[node_index.val:: UInt16] |
|
Besides the ones mentioned in the other issues at the one in the comment above are there any other ones in the diff that are also quirks? |
|
I'm seeing several new kinds: open(path, "w") do io
- TOML.print(io, data)
+ return TOML.print(io, data)
end node_sample_size =
- rand(rng, 1:(nv(base_proposal.grid_graph) - length(base_proposal.rez_assets))) - 1
+ rand(rng, 1:(nv(base_proposal.grid_graph)-length(base_proposal.rez_assets))) - 1- known_nodes = known_nodes[.!to_purge, :]
+ known_nodes = known_nodes[.! to_purge, :] [:FrEnode, :ToEnode] =>
(FrEnode, ToEnode) ->
(FrEnode == from.id && ToEnode == to.id) ||
- (FrEnode == to.id && ToEnode == from.id),
+ (FrEnode == to.id && ToEnode == from.id),
branches,- branches.Foooooooo[to_impute] .=
- get_foooooooo.(branches.Voltage[to_impute], branches.Distance[to_impute])
+ branches.Foooooooo[to_impute] .= get_foooooooo.(
+ branches.Voltage[to_impute], branches.Distance[to_impute]
+ ) |
|
this one is actually more correct now since the indentation should only be applied to shortcircuit boolean expressions. In the example above it's a regular boolean expression where a true of false is returned whereas in a shortcircuit expression something else (although it can also be a boolean) is returned or done once the initial calls up to the last one are completed, i.e. |
|
I was reviewing the changes in v2 and found a quirk (only a single one actually, so very good job there! 😉 ), so I though I would report it here instead of opening another issue about the same. In short I think there's unstable formatting at https://github.com/COBREXA/COBREXA.jl/blob/ed2e2601b5f1bff04f9927e660c587b1ba52286b/src/frontend/balance.jl#L68-L74 (the source is currently formatted by JuliaFormatter 1): By default JuliaFormatter2 inserts an (slightly under-explained) empty line there, as such: met => C.Constraint(
value = let i = stoiT.colptr[row_idx], e = stoiT.colptr[row_idx+1] - 1
C.LinearValue(idxs = stoiT.rowval[i:e], weights = stoiT.nzval[i:e])
end,
bound = C.EqualTo(b),
) for (met, row_idx, b) in zip(mets, 1:stoiT.n, bal)OTOH, when I move the any of the let-definitions to the other line, JuliaFormatter2 seems to converge to a different formatting: met => C.Constraint(
value = let
i = stoiT.colptr[row_idx], e = stoiT.colptr[row_idx+1] - 1
C.LinearValue(idxs = stoiT.rowval[i:e], weights = stoiT.nzval[i:e])
end,
bound = C.EqualTo(b),
) for (met, row_idx, b) in zip(mets, 1:stoiT.n, bal)I.e., both of these seem to be accepted as "valid" and stable by JF, while the latter is clearly better. (Note: This is with version from current |
|
PS @kenahoo I'm seeing similar as you do but since these seem consistent and converge to a single style I thought these are actually on purpose and OKish. In short it seems to me that 1] stuff in square brackets is no longer compacted by removing spaces around operators by default 2] stuff in range expressions with |
|
@exaexa these are actually different in terms of how they scope the variables; see https://docs.julialang.org/en/v1/manual/variables-and-scoping/#Let-Blocks. Putting a variable assignment on the same line as a |
|
Best summarised like this: |
EDIT: deleted the previous speculation, apologies for the noise, thanks for explanation. Still, is there an explanation for the blank line in this format? :flux_stoichiometry^C.ConstraintTree(
met => C.Constraint(
- value = let i = stoiT.colptr[row_idx],
- e = stoiT.colptr[row_idx+1] - 1
+ value = let i = stoiT.colptr[row_idx], e = stoiT.colptr[row_idx+1] - 1
+
C.LinearValue(idxs = stoiT.rowval[i:e], weights = stoiT.nzval[i:e])
end,
bound = C.EqualTo(b),
) for (met, row_idx, b) in zip(mets, 1:stoiT.n, bal)
) |
|
I’m assuming it’s just to help disambiguate between let x = 1,
y = 2
endWhere the let x = 1
y = 2
endwhere the So I could see how the let x = 1
y = 2
endIs more clear about this distinction |
|
@MilesCranmer good point, it's probably some logic failure around the decision on when to add the extra empty line there. I guess the logic there is subtle. While testing the gazillion of possible combinations I noticed that v2 doesn't add spaces around the equalsigns in let x=very(long, thing, to, fit, on, one, line), # the line gets split here
y = another(long, thing, to, fit, easily, to, a, single, lone)
# this empty line is inserted just right
z = x(y(z, also, quite, long, not, sure, how, much, or, what))
endAnyway I found that the empty line in the following shortened snipped is actually retained only if let x=very(long, thing, to, fit, on, one, line), y = another(long, thing, to, fit)
z = x(y(z, also, quite, long, not, sure, how, much, or, what))
endMaybe, is there some option to just ignore all source indentation and formatting and reconstruct the files from a completely space-less AST? |
|
Hm I'm not sure about that one |
|
this is the section that adds the newline. It's added there to let to help differentiate the "let" arguments from the "let" body. This also applies to for loops. I personally don't have a preference either way. The extra line is only added if the arguments or body is nested, which in the example above doesn't seem to be the case so that might be a bug. |
|
https://github.com/domluna/JuliaFormatter.jl/blob/master/src/print.jl#L189-L194 This is where remove extra newlines is used. It's possible there is some odd interaction here since it doesn't factor in Newline() nodes, meaning if there is a Newline and a Notcode immediately follows then perhaps there will still be extra newlines. |
|
@domluna thanks! I'm actually not super interested in whether the newline is or isn't there, the main concern for me is to make sure that the input spaces (outside strings&comments ofc, and [edit:] the explicit empty-line spaces) have no effect on the output. This looked like it's somewhat input-spacing-dependent, with the newline removal option it looks OK. Btw it might be super useful for people who use this for technical purposes (commit cleaning in my case) to have the "ignore all input spaces" option explicitly available. |
Just pasting the diffs of some of my packages to help identify new formatting issues. These overlap with #878 and #879. I’m using BlueStyle.
As you can see, many of these are superficial changes (though some be violating Blue, not sure). However, some of these formatting changes are semantic differences like #878.
Here's the diff for SymbolicRegression.jl
The text was updated successfully, but these errors were encountered: