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

Alternative specification for pipe formatting #60

Open
CabalCrow opened this issue May 26, 2024 · 13 comments
Open

Alternative specification for pipe formatting #60

CabalCrow opened this issue May 26, 2024 · 13 comments
Labels
enhancement New feature or request

Comments

@CabalCrow
Copy link

CabalCrow commented May 26, 2024

The specification.md describes the default formatting for pipelines to be a wall of lines.

def "apply to" [
    file: path
    modification: closure
] {
    $file
    | path expand
    | open --raw
    | from toml
    | do $modification
    | save --force $file
}

In the nushell discord fdncred mentioned how this creates issues with copy pasting code to the terminal on windows. Because of this I would like to suggest alternative formatting & indenting:

def "apply to" [
    file: path
    modification: closure
] {
    $file |
        path expand |
        open --raw |
        from toml |
        do $modification |
        save --force $file
}

This python style indenting still showcases that a pipeline is continued, despite that the pipe symbol is not at the start of the line.

@CabalCrow CabalCrow added the enhancement New feature or request label May 26, 2024
@fdncred
Copy link
Collaborator

fdncred commented May 26, 2024

Agreed.

@AucaCoyan
Copy link
Contributor

Hi! Yes, this is really useful. When I wrote some lines in the specification I though this as a parameter "do you want the pipelines to be connected?" and exactly these scripts are the perfect example 😊

@amtoine
Copy link
Member

amtoine commented May 31, 2024

i think this should be modifiable in some settings, i personally quite strongly dislike the second way 🤣

and i would write the following instead

def "apply to" [
    file: path
    modification: closure
] {
    $file
        | path expand
        | open --raw
        | from toml
        | do $modification
        | save --force $file
}

@CabalCrow
Copy link
Author

i think this should be modifiable in some settings, i personally quite strongly dislike the second way 🤣

and i would write the following instead

def "apply to" [
    file: path
    modification: closure
] {
    $file
        | path expand
        | open --raw
        | from toml
        | do $modification
        | save --force $file
}

This falls into the same issue for windows users - you can't have a | at the start of the line, indent or not.

@amtoine
Copy link
Member

amtoine commented Jun 1, 2024

@CabalCrow

'tis why i propose to make in configurable :)

@CabalCrow
Copy link
Author

@amtoine Having it configurable, is good, but I'm also talking about what the default formatting should be. I believe that you would want default formatting that could work properly regardless of OS, since nushell aims to work regardless of OS.

@AucaCoyan
Copy link
Contributor

This falls into the same issue for windows users - you can't have a | at the start of the line, indent or not.

What do you mean with Windows users? I can copy and paste that script in my nushell terminal and it works 🤩. Do you have trouble with that script?

@amtoine
Copy link
Member

amtoine commented Jun 2, 2024

yeah i think Nushell should be able to handle copy pasting any way to format pipelines, the parser shouldn't care at all 😕

if that's not possible, i agree the default should work the best for everyone -even though it's Windows which is annoying again, ugh...-, i would just turn that default off as soon as i start a Nushell project 😉

my gut feeling is that, putting the | at the end of lines would like doing the following in Rust

            let vals = rec.
                values().
                cloned().
                enumerate().
                map(|(i, v)| {
                    if i == id {
                        mutate_value_cell(&v, &cell_path, cell).unwrap()
                    } else {
                        v
                    }
                }).
                collect();

instead of the conventional

            let vals = rec
                .values()
                .cloned()
                .enumerate()
                .map(|(i, v)| {
                    if i == id {
                        mutate_value_cell(&v, &cell_path, cell).unwrap()
                    } else {
                        v
                    }
                })
                .collect();

anyways 😉

@amtoine
Copy link
Member

amtoine commented Jun 2, 2024

@AucaCoyan
if there is no issue anymore with copy pasting pipelines on Windows, then it's all good 😌

@CabalCrow
Copy link
Author

CabalCrow commented Jun 2, 2024 via email

@fdncred
Copy link
Collaborator

fdncred commented Jun 2, 2024

Here is an example @AucaCoyan, if your terminal does not support bracketed paste, such as Windows Terminal, when you paste the middle portion of the custom command, something like this

    $file
        | path expand
        | open --raw
        | from toml
        | do $modification
        | save --force $file

nushell/reedline pastes one line at a time. So, it looks like this to nushell and each line tries to be executed giving you a different result than what you want.

$file # it runs this
    | path expand # then tries to run this
    | open --raw # then tries to run this, and so on

alternatively, if you paste this

    $file |
        path expand |
        open --raw |
        from toml |
        do $modification |
        save --force $file

nushell sees

$file | # oh, this statement isn't complete, don't execute it yet
    path expand | # oh, there's another pipe, so this statement isn't complete, don't execute yet
   open --raw | # oh, another pipe, don't execute yet

So, all I was saying is that I prefer the pipes at the end because Windows users won't have a problem with copying and pasting pieces out of the script/custom command. You are correct that the entire custom command can be pasted fine but that's because it's in a { } block but if you try to play with just the middle pieces, it won't work.

You can see here that I pasted the entire custom command and it worked just fine. But if I want just the part in question, it pukes because the pipes are in the wrong place for windows to understand what to do.
image
But if the pipes are at the end, windows has no issues with it
image

@AucaCoyan
Copy link
Contributor

Thanks for the clarification! I tried also in Windows and had the same result in Windows Terminal. Because I've been using nushell in windows for so long I didn't expect it to "magically work" in linux 😋.
BTW, is there any other terminal in windows that supports bracketed paste? Wezterm and Rio also couldn't resolve the job

@fdncred
Copy link
Collaborator

fdncred commented Jun 2, 2024

I'm not sure. I'm hopeful that once Ghostty gets fully supported in Windows that it will be I'm not sure. I think in most terminals you have to enable the feature though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants