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

Design for executing either script or file #21

Open
erick-thompson opened this issue Oct 4, 2024 · 1 comment
Open

Design for executing either script or file #21

erick-thompson opened this issue Oct 4, 2024 · 1 comment

Comments

@erick-thompson
Copy link

erick-thompson commented Oct 4, 2024

I was looking at issue #11 which was attempted to be fixed with #20. The change wasn't landed because of the difference in launching PowerShell file and a script block. My particular use case is mainly around files with parameters, so I'm looking at how this would work.

What do you think about a builder approach where you need to specify script or file, which can then allow for args only when you're going to run a file?

If this seems acceptable, I'd be happy to work on a PR

Something like

fn main() {
    let ps = PsScriptBuilder::new()
        .no_profile(true)
        .non_interactive(true)
        .hidden(false)
        .print_commands(false)
        .file("path/to/file.ps1")
        .arguments.add("argument")
        .build();
    // below fails
    let output = ps.run(r#"echo "hello world""#).unwrap();        
    
    // below works
    let output = ps.run().unwrap();
}

likewise

fn main() {
    let ps = PsScriptBuilder::new()
        .no_profile(true)
        .non_interactive(true)
        .hidden(false)
        .print_commands(false)
        .build();

    // below works
    let output = ps.run(r#"echo "hello world""#).unwrap();        
    
    // below fails
    let output = ps.run().unwrap();
}

I think it would be better to have both approaches specify the payload (file or script) in the builder methods, but that would conflict with how it works currently.

@cfsamson
Copy link
Owner

cfsamson commented Dec 4, 2024

Hi and sorry for the delayed response.

I think this might be a good idea. I think this will be a breaking change anyway, but if we end up with a better API I think it's worth it. Since there seems to be quite a bit of interest for this I think the solution you suggest seems fine and I would really appreciate any help I can get on this.

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

No branches or pull requests

2 participants