Skip to content

Fix variables not allowed in ucp#10304

Merged
fdncred merged 5 commits intonushell:mainfrom
dmatos2012:ucp_fix_vars_in_paths
Sep 10, 2023
Merged

Fix variables not allowed in ucp#10304
fdncred merged 5 commits intonushell:mainfrom
dmatos2012:ucp_fix_vars_in_paths

Conversation

@dmatos2012
Copy link
Copy Markdown
Contributor

Description

Fixes #10300 , where using variables didnt work with ucp as it was only expecting a Expr::FilePath.

Before: (from the issue)

❯ ucp -r $var $folder
Error:   × Missing file operand
   ╭─[entry #40:1:1]
 1 │ ucp -r $var $folder
   · ─┬─
   ·  ╰── Missing file operand
   ╰────
  help: Please provide source and destination paths

Now:

`ucp -r $var $folder`
# success

Also added the test to ensure its working:) . Oh, and I tweaked again slightly the messages on two tests because now the whole path is printed rather than a. Say:

#before
`cp a a` --> 'a' and 'a' are the same file 
# now
`cp a a` --> /home/current/location/a and /home/current/location/a are the same file

User-Facing Changes

Tests + Formatting

Don't forget to add tests that cover your changes.

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 --workspace -- -D warnings -D clippy::unwrap_used to check that you're using the standard code style
  • cargo test --workspace to check that all tests pass (on Windows make sure to enable developer mode)
  • cargo run -- -c "use std testing; testing run-tests --path crates/nu-std" to run the tests for the standard library

After Submitting

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 10, 2023

Thanks @dmatos2012. Let's try this out!

@fdncred fdncred merged commit ce378a6 into nushell:main Sep 10, 2023
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 10, 2023

@amtoine I think this fixes the problem you were seeing in your script. Can you try it out and let me know how it goes? I can't find our discord chat where you reported this problem.

@amtoine
Copy link
Copy Markdown
Member

amtoine commented Sep 11, 2023

i'm building my terminal and then Nushell, will come back shortly ♻️

@amtoine
Copy link
Copy Markdown
Member

amtoine commented Sep 11, 2023

@fdncred @dmatos2012
looks like i cannot reproduce anymore, good job 🙏

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 11, 2023

yay! good work David!

@dmatos2012
Copy link
Copy Markdown
Contributor Author

Yay thanks 🙏! Let's keep those bug reports coming 😂

@dmatos2012 dmatos2012 deleted the ucp_fix_vars_in_paths branch September 11, 2023 19:14
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.

ucp doesn't like using variables as source and destination

3 participants