Deleting commas in parameter lists makes one-line function args hard to read

[TLATER]

This is deliberate, but I think the result is not very readable:

@@ -1,6 +1,6 @@
 use std/log
 
-def main [interface_arg: string, action_arg?: string] {
+def main [interface_arg: string action_arg?: string] {
   let action = $action_arg | default $interface_arg
   let interface = match $interface_arg {
     none => null

I don't mind this for long parameter lists that are spread on multiple lines, but for these kinds of mini-functions with self-explaining args in little scripts I would really like to keep my commas.

[fdncred]

i could live with it either way but commas are not required in nushell, and in fact we sometimes discourage them because it's just extra typing. so, if we're formatting for nushell, without commas is correct. maybe there's a way to make it optional whether to keep commas or not?

[TLATER]

I don't see why it would be discouraged in this specific scenario, it's useful as a text marker for the same reason commas are useful in natural languages - a visual indicator for "last thing has stopped here".

Again, I don't at all mind omitting commas for declarations like this:

[
  interface_arg: string  # Some doc comment
  action_arg?: string    # Some other doc comment
]

In that case the newlines are even better to distinguish the args, and the commas are indeed superfluous and in fact a source of merge conflicts, so I think it's good to omit them there.

It's just visually hard to figure out what's happening here:

[interface_arg: string action_arg?: string]

... and a linter really ought not to make things harder to read.

For non-parameter lists this isn't much of a problem, since the items are already visually distinct:

["one" "two" "three"]

It's the addition of the type markers that causes issues here.

---

maybe there's a way to make it optional whether to keep commas or not?

If this is desired I'd suggest keeping with topiary's rule of "do what the user meant to do" from indentation, and omit the commas if the first isn't set, but add them if the first is set. I.e.:

-[interface_arg: string action_arg: string, third_arg: string]
+[interface_arg: string action_arg: string third_arg: string]

and:

-[interface_arg: string, action_arg: string third_arg: string]
+[interface_arg: string, action_arg: string, third_arg: string]

But personally I'd really prefer just enforcing commas for un-indented parameter lists, and enforce removing them for indented parameter lists; read the above two diffs, at least to me the former is really hard to parse visually, and I'm pretty sure this will be true for most humans.

Unlike with indented parameter lists, it doesn't dodge any merge conflicts either. The extra typing is a bit of a moot point since you're going to be autoformatting anyway if you use topiary, so the commas are added automatically. It's just a net reduction in readability. Formatting is of course always subjective, but I don't think anyone should be writing parameter lists like this.

[ItsHarper]

I couldn't agree more with everything TLATER has said

[fdncred]

ya, there's a lot of personal preferences in code formatting. i was just explaining the nushell point of view.

lists like this ["one" "two" "three"] also don't need quotes. another nushell-ism.

blindFS will do what he thinks is right for his project.

[blindFS]

There is this PR #34, still not merged because

1. It's a huge breaking change, need some volunteers to test it so I can make sure it's robust enough (might need some updates to work with the latest ts-nu parser).
2. It introduces more dirty hacks because of one single bug of tree-sitter, I once tried to fix it, but it's trickier than I thought.
3. There was a rumored WIP topiary feature that will set style preferences according to the first occurrence of that pattern in the file.
4. The official nufmt has been greatly improved recently, maybe that's a better choice for more style options.

[TLATER]

Ah, thanks, my bad, I skimmed past the draft PRs.

I did notice the nufmt improvements after creating this thread, and it no longer has the "pre-alpha" warning, so it might indeed be worth switching to.

[blindFS]

@TLATER

Yeah, I'd definitely suggest you try it, eventually this project will be archived when nufmt is robust enough.