Alright, we're here - the first numbered release of nph is here!
In case you didn't read the massive thread introducing the project, nph is an opinionated code formatter for Nim. It will take Nim code and give it a consistent style no matter how it was was formatted before - for examples, see the other post.
Taking feedback from that post, the formatter has gone through several rounds of improvements and fixes and it's looking really good - so good that whatever imperfections there may be, they are far outweighed by improved consistency that comes with automated formatting and the relief at not having to realign that variable for the 100th: time. Give it a few weeks and you will not regret using it.
For the next version, I'm planning on documenting the style a bit more along with some rationale for the choices made, but broadly the formatter will consider several options and pick the best formatting based on a few priorities such as the avoidance of wasted space balanced with structural clarity of the code.
In terms of features, since the initial post several things have happened:
In the introduction post, I mentioned I will not format code manually again. Indeed, nph is itself formatted with nph and this is checked by github actions - to celebrate 0.2, nim-results also got an nph make-over ;)
More examples include the compiler and nimbus-eth2.
I would generally not recommend it yet for large projects like that which have lots of code and history mostly because the style is not yet stable - ie there are still improvements that can be made that are likely to result in some reformatting happening - that said, it's a good moment to use it for a bit on smaller scale and see where the friction points are, ie what choices you will get used to / are a function of your habit and which are genuine problems that keep itching.
The project is now also tracking the formatting of these projects so that changes to the formatting can be verified to make sense: https://github.com/arnetheduck/nph/blob/5749abc338c058b9a54e801aa8a5ab706f19ff14/nph.nimble#L45
If you end up using nph for a public project, feel free to reach out and I can add it to that list for visibility.
2.0.0 is for compiling the project - you can use it with your other code.
If you have the latest nimble installed, it should install the right version for you.
IMHO, the linked FAQ entry doesn't give a satisfactory answer. I get it that consistency is an important goal,. But IMO this should apply to a project's or organisation's code base, not the whole world's Nim codebase. We're not Borg and different code bases have different requirements.
But it's fine, I don't have to agree with nph's goals and can choose not to use it.
But it's fine, I don't have to agree with nph's goals and can choose not to use it.
You can also fork it and build a max-options variant off it, which is probably more productive ;) The codebase is easy and I'm happy to make it easier for anyone that wants to.
I'm curious though, what options specifically would you want to see? Or is it just on principle, that there should be options, preferably lots?
In principle there should be a single switch named --detect that detects the indentation width, the choice of comma vs semicolon and other controversial issues. Then the "my way or the highway" people can use the tool without the --detect switch so that everything is rewritten to the one way nph's author prefers but others can use it too.
For every controversial issue, there should be documented how --detect detects it. For example, it could use semicolons if some proc declaration in the source code file uses semicolons.
decisions and coding guidelines are an important step
indeed - "use nph" takes about 2s to write down, then you can productively move on to more important decisions ;)
Is planned something like https://pycqa.github.io/isort/ in nph ?
For example, it could use semicolons if some proc declaration in the source code file already uses semicolons.
The semantic difference between using them and not using them makes this kind of detection fickle at best - there are many edge cases and irregularities in the parser - ditto can be said of most "controversial" cases, ie making something that works (ie doesn't break on unusual cases) and guesses the intentions right makes for complex code.
It's sort of possible to reverse engineer the parser and the strange quirks that have been added over time and create a use-commas-where-possible rule but that adds quite a bit of complexity and results in code that sometimes for (to mere mortals) inexplicable reasons inserts ; - I'm somewhat hesitant for this reason.
More broadly, it defeats the purpose of the tool (which is to focus on efficient collaboration across the ecosystem for participating projects).
funny because I just found a place where ; cannot be parsed correctly in a parameter list:
type
SwitchCreator =
proc(
ma: MultiAddress = MultiAddress.init("/ip4/127.0.0.1/tcp/0").tryGet(),
prov: TransportProvider =
proc(upgr: Upgrade): Transport =
TcpTransport.new({}, upgr)
,
relay: Relay = Relay.new(circuitRelayV1 = true),
): Switch {.gcsafe, raises: [LPError].}
change , to ; and there's no way to inline the proc impl that I can find. I suspect the ; ends up being parsed by the proc parser.
Conclusion: we're back to using , as separator except in the case of missing type/default (as sometimes happens in generics and templates), where we use ; to retain AST equivalence of identifier groups.
Sorry, I don’t understand why using ; is not possible for identifiers with defaults. The following example is perfectly valid and I don’t see any parsing problem:
proc p(a: int; b: char; c, d = 3; e = 4) = discard
Am I missing something? Is there any problem with the AST in this case?
As regards consistency, I don’t see why we should use the same separator for parameters in function/procedure definitions and in function/procedure calls, since a parameter list in a procedure definition is syntactically totally different of a parameter list in a procedure call.
In fact, Nim is not the first language to encounter this problem. Several languages use semicolons as separator in procedure definition, for instance Pascal, Modula family, Ada.
If people tends to use colons rather than semicolons, this is not because this is more rational or more consistent but rather that many people have practiced languages such as C, C++, Python, Ruby, etc., which use a comma as separator. However, as Nim's syntax is closer to that of languages such as Pascal (particularly in the definition of parameters), I think that the semicolon makes more sense.
It's not that I'm advocating replacing commas with semicolons, but as the semicolon is more rational and more legible, I think it's only fair to respect the choice of those who have adopted it.
Am I missing something? Is there any problem with the AST in this case?
Read (and test) the post above with the proc in it.
only fair to respect the choice of those who have adopted it.
Happy comma users, happy semicolon users, opinionated formatter - pick any 2.
Semi-colons are indeed a left-over from languages such as Pascal - their main feature seems to be the enablement of style debates ;)
Sorry, I should have taken a better look at the previous posts 🙁.
For me, it looks like a bug in the parser. As you said, it seems that the parser has swallowed the semicolon when processing the inner proc.
Using a modified example with only standard types, I found two workarounds: either add an extra semicolon to terminate the inner proc definition or put the whole inner proc definition between parentheses. In both cases, this helps the parser to understand that the definition is terminated.
Of course, these are only workarounds.
Regarding nph, we have indeed been warned: it is opinionated, so I understand that it is impossible to satisfy everybody. But, anyway, I think that style debate is inevitable for a code formatter 🙂.