nushell / nufmt

MIT License
64 stars 8 forks source link

`--stdin` doesn't read stdin #56

Open dead10ck opened 8 months ago

dead10ck commented 8 months ago

I'm not sure if I'm just missing something here, but it seems that the --stdin flag requires an argument, which is supposed to be a literal nushell script. That's not what standard input means, and limits the size of the script to the system's maximum command size length.

I would expect the --stdin flag to make the formatter... read from stdin.

amtoine commented 8 months ago

yeah, should probably allow

open foo.nu | nufmt --stdin

instead of

nufmt --stdin (open foo.nu)

, right?

dead10ck commented 8 months ago

Right. I think taking a literal nushell script as a command line argument doesn't make any sense. Positional arguments and flags are not intended for such long input.

AucaCoyan commented 8 months ago

Hi! you are correct, stdin requires an argument, I started working a bit mindless-ly and thought:

Well, if it doesn't require a flag to format, should nufmt format all files in the folder or format the upcoming input stream?

Maybe we can follow this logic

  1. for the only nufmt command, show the help
  2. for nufmt --stdin may take an input 2.1. if something comes from an input, format that, so
    open foo.nu | nufmt --stdin

    would work 2.2 if nothing comes from stdin, then you can pass a string as an argument "let my_var = 3;" 2.3 if nothing comes from stdin or there is no argument, error out and show the help "I need some input with the --stdin flag"

What do you think?

dead10ck commented 8 months ago

Passing a value to a flag named "stdin" doesn't make any sense—in this case, it's not standard input, it's just a positional argument.

But moreover, I don't think positional arguments of any kind make any sense. There's a limit of 4k for an exec; passing anything but the most trivial of scripts is going to fail.

AucaCoyan commented 8 months ago

I'm not sure if I follow you.

What could solve this issue, or what is the expected behavior you would like to see?

amtoine commented 8 months ago

@AucaCoyan i think this is the expected behaviour, i.e.

open foo.nu | nufmt --stdin
AucaCoyan commented 8 months ago

Thanks! I will head towards it

alecandido commented 3 months ago

Since this is taking a while, here is a very temporary workaround:

#!/usr/bin/env -S nu --stdin
nufmt --stdin $in | tail -n +2

I'd like to help myself to speed up, but I'm not sure I will have enough time soon. I just thought that since this is enough for me, to use nufmt inside Neovim, it could be beneficial even for anyone else arriving here.

amtoine commented 3 months ago

@alecandido help would be much apreciated :pray:

are you using nufmt for real in projects? :open_mouth:

alecandido commented 3 months ago

help would be much apreciated 🙏

I'd be glad to give it, I'm just short of time. But I'll definitely take it into account.

are you using nufmt for real in projects? 😮

I'm just addicted to formatters. As soon as I started writing scripts (in particular, just config.nu, and some trials) I started looking for something saving me the burden of thinking about irrelevant style.

The result of nufmt is not great, but still usable. I could wish something better, but if I'll desire it enough I might be motivated to contribute :)

fdncred commented 3 months ago

We would love to have more contributors to this repository. We'd all love to have a good nushell script formatter.