Skip to content

Fix quoting for command line args#5384

Merged
fdncred merged 3 commits intonushell:mainfrom
uasi:fix-nu-c
Apr 30, 2022
Merged

Fix quoting for command line args#5384
fdncred merged 3 commits intonushell:mainfrom
uasi:fix-nu-c

Conversation

@uasi
Copy link
Copy Markdown
Contributor

@uasi uasi commented Apr 30, 2022

Description

Fixes #5370
Fixes #5371

Tests

Make sure you've run and fixed any issues with these commands:

  • cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
  • cargo clippy --all --all-features -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo build; cargo test --all --all-features to check that all the tests pass

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Apr 30, 2022

seems a reasonable approach. let's see how it works. Thanks!

@fdncred fdncred merged commit ae9c0fc into nushell:main Apr 30, 2022
"--commands" | "-c" => {
// FIXME: Use proper quoting. `escape_quote_string()` can't be used for now due to https://github.com/nushell/nushell/issues/5383.

args.next().map(|a| format!("`{}`", a))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using proper quoting will fix #5175

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to propose a 'proper quoting' fix too. :)

@uasi uasi deleted the fix-nu-c branch April 30, 2022 18:56
@elferherrera
Copy link
Copy Markdown
Contributor

elferherrera commented May 1, 2022

I think this PR caused this issue

image

I used the next command

 cargo run -- --config ..\config.nu  

@uasi uasi mentioned this pull request May 1, 2022
3 tasks
@sophiajt
Copy link
Copy Markdown
Contributor

sophiajt commented May 1, 2022

@elferherrera I just landed #5398. Can you see if it fixes your issue?

@elferherrera
Copy link
Copy Markdown
Contributor

it seems that it didnt work
image

@sophiajt
Copy link
Copy Markdown
Contributor

sophiajt commented May 1, 2022

@elferherrera this should now be fixed in the latest main. Took a few tries, but I think I found it

fennewald pushed a commit to fennewald/nushell that referenced this pull request Jun 27, 2022
* Fix quoting for command line args

* Replace custom quoting with escape_quote_string

* Use raw string for now
@fdncred fdncred mentioned this pull request Jul 28, 2022
6 tasks
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

Successfully merging this pull request may close these issues.

nu script.nu '"' fails nu -c "" fails

4 participants