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

Arguments Should Not Be Slit on Spaces #10

Closed
Vehmloewff opened this issue Nov 6, 2023 · 3 comments
Closed

Arguments Should Not Be Slit on Spaces #10

Vehmloewff opened this issue Nov 6, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@Vehmloewff
Copy link
Owner

Vehmloewff commented Nov 6, 2023

Pardon me, dear users of this extension... 3 years ago myself was a bit of a noob.

This is bad:

const args = command.replace(/\$FILE/g, filename).split(' ')

https://github.com/Vehmloewff/custom-format/blob/master/src/run-command.ts#L6

Users expect to be able to quote an argument with spaces in it (or escape the spaces with \) so that it will be treated as a single argument.

This, however, is out of the scope of this tool, and better handled by the user's shell.

I propose that we deprecate command, and add the following three settings to a formatter:

  • file: string - execute a single file
  • args: string[] - arguments to be passed to file when executing it
  • sh: string - a shorthand for setting file to the user's default shell and args to the platform equivalent of ['-c', '...']. The behavior would be similar to the way that Dtils' sh function gets it's arguments.

It would be illegal to provide both sh and file/args.

Additionally, I propose we change $FILE from a fancy string replacement to an environment variable.

@Vehmloewff Vehmloewff added the bug Something isn't working label Nov 6, 2023
@Vehmloewff
Copy link
Owner Author

Fixed in the latest release

@JimmyZJX
Copy link

I might miss something but how is this issue resolved? I cannot use the new options file, args or sh in the latest version (1.0.5).

@Vehmloewff
Copy link
Owner Author

Hello @JimmyZJX!

I went ahead and decided to treat command like the above specifies that sh should be implemented. I didn't think about it before, but that is a non-breaking solution that doesn't require the addition of extra configuration options.

I labeled it as "fixed" because the command is now being split up into arguments correctly, but I realize now that I neglected to here note my deviation from the stated plan. I'm sorry for the inconvenience and confusion that this probably caused you.

That said, I'm assuming that you got to this GitHub issue because you were having some trouble. Are arguments being split incorrectly for you?

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

No branches or pull requests

2 participants