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

--stdin doesn't read stdin #56

Open
dead10ck opened this issue Dec 17, 2023 · 12 comments
Open

--stdin doesn't read stdin #56

dead10ck opened this issue Dec 17, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@dead10ck
Copy link

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.

@dead10ck dead10ck added the bug Something isn't working label Dec 17, 2023
@amtoine
Copy link
Member

amtoine commented Dec 17, 2023

yeah, should probably allow

open foo.nu | nufmt --stdin

instead of

nufmt --stdin (open foo.nu)

, right?

@dead10ck
Copy link
Author

dead10ck commented Dec 17, 2023

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
Copy link
Contributor

AucaCoyan commented Dec 17, 2023

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
Copy link
Author

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.

@amtoine amtoine linked a pull request Dec 17, 2023 that will close this issue
@AucaCoyan
Copy link
Contributor

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
Copy link
Member

amtoine commented Dec 18, 2023

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

open foo.nu | nufmt --stdin

@AucaCoyan
Copy link
Contributor

Thanks! I will head towards it

@alecandido
Copy link

alecandido commented Apr 30, 2024

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
Copy link
Member

amtoine commented May 1, 2024

@alecandido
help would be much apreciated 🙏

are you using nufmt for real in projects? 😮

@alecandido
Copy link

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
Copy link
Collaborator

fdncred commented May 1, 2024

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

fdncred pushed a commit that referenced this issue Jul 15, 2024
## Description of changes

`--stdin` flag read a command line argument, not stdin. Now it reads
from stdin.

Not sure if my code would compile on windows, but I don't have a windows
computer to test it.

I'm aware that #57 tries to solve
this as well, but it seems stagnant and won't pass CI.

## Relevant Issues

#56
@AucaCoyan
Copy link
Contributor

#61 solves the bug, it correctly reads from stdin with the flag

"let one = 1;" | nufmt --stdin

Is it okay to close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants