Mirrors/nushell
2
0
Fork 0
mirror of https://github.com/nushell/nushell synced 2025-01-13 21:55:07 +00:00
Code Issues Projects Releases Packages Wiki Activity

Small fixes and refactors to paths & source command (#3998)

Browse source 
* Expand path when converting value -> PathBuf

Also includes Tagged<PathBuf>.

Fixes #3605

* Expand path for PATH env. variable

Fixes #1834

* Remove leftover Cows after nu-path refactor

There were some unnecessary Cow conversions leftover from the old
nu-path implementation.

* Use canonicalize in source command; Improve errors

Previously, `source` used `expand_path()` which does not follow
symlinks.

As a follow up, I improved the source error messages so they now tell
why the source file could not be canonicalized or read into string.
This commit is contained in:
Jakub Žádník 2021-09-12 02:36:14 +03:00 committed by GitHub
parent 0fa0c25fb3
commit cc5c4d38bb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 55 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
crates/nu-command/src/commands/core_commands/source.rs
View file

@ -2,11 +2,11 @@ use crate::prelude::*;
use nu_engine::{script, WholeStreamCommand};
use nu_errors::ShellError;
use nu_path::expand_path;
use nu_path::{canonicalize, canonicalize_with};
use nu_protocol::{Signature, SyntaxShape};
use nu_source::Tagged;
use std::{borrow::Cow, path::Path, path::PathBuf};
use std::path::Path;
pub struct Source;
@ -69,11 +69,15 @@ pub fn source(args: CommandArgs) -> Result<OutputStream, ShellError> {
for lib_path in dir {
match lib_path {
Ok(name) => {
let path = PathBuf::from(name).join(source_file);
let path = canonicalize_with(&source_file, name).map_err(|e| {
ShellError::labeled_error(
format!("Can't load source file. Reason: {}", e.to_string()),
"Can't load this file",
filename.span(),
)
})?;
if let Ok(contents) =
std::fs::read_to_string(&expand_path(Cow::Borrowed(path.as_path())))
{
if let Ok(contents) = std::fs::read_to_string(path) {
let result = script::run_script_standalone(contents, true, ctx, false);
if let Err(err) = result {
@ -89,9 +93,15 @@ pub fn source(args: CommandArgs) -> Result<OutputStream, ShellError> {
}
}
let path = Path::new(source_file);
let path = canonicalize(source_file).map_err(|e| {
ShellError::labeled_error(
format!("Can't load source file. Reason: {}", e.to_string()),
"Can't load this file",
filename.span(),
)
})?;
let contents = std::fs::read_to_string(&expand_path(Cow::Borrowed(path)));
let contents = std::fs::read_to_string(path);
match contents {
Ok(contents) => {
@ -102,10 +112,10 @@ pub fn source(args: CommandArgs) -> Result<OutputStream, ShellError> {
}
Ok(OutputStream::empty())
}
Err(_) => {
Err(e) => {
ctx.error(ShellError::labeled_error(
"Can't load file to source",
"can't load file",
format!("Can't load source file. Reason: {}", e.to_string()),
"Can't load this file",
filename.span(),
));

4
crates/nu-command/src/commands/path/expand.rs
View file

@ -5,7 +5,7 @@ use nu_errors::ShellError;
use nu_path::{canonicalize, expand_path};
use nu_protocol::{ColumnPath, Signature, SyntaxShape, UntaggedValue, Value};
use nu_source::Span;
use std::{borrow::Cow, path::Path};
use std::path::Path;
pub struct PathExpand;
@ -105,7 +105,7 @@ fn action(path: &Path, tag: Tag, args: &PathExpandArguments) -> Value {
tag.span,
))
} else {
UntaggedValue::filepath(expand_path(Cow::Borrowed(path))).into_value(tag)
UntaggedValue::filepath(expand_path(path)).into_value(tag)
}
}

3
crates/nu-completion/src/path.rs
View file

@ -1,4 +1,3 @@
use std::borrow::Cow;
use std::path::{is_separator, Path, PathBuf};
use super::matchers::Matcher;
@ -27,7 +26,7 @@ impl PathCompleter {
(base, rest)
};
let base_dir = nu_path::expand_path(Cow::Borrowed(Path::new(&base_dir_name)));
let base_dir = nu_path::expand_path(&base_dir_name);
// This check is here as base_dir.read_dir() with base_dir == "" will open the current dir
// which we don't want in this case (if we did, base_dir would already be ".")
if base_dir == Path::new("") {

3
crates/nu-engine/src/evaluation_context.rs
View file

@ -10,6 +10,7 @@ use crate::{env::basic_host::BasicHost, Host};
use nu_data::config::{self, Conf, NuConfig};
use nu_errors::ShellError;
use nu_path::expand_path;
use nu_protocol::{hir, ConfigPath, VariableRegistry};
use nu_source::Spanned;
use nu_source::{Span, Tag};
@ -157,7 +158,7 @@ impl EvaluationContext {
for (var, val) in env_vars {
if var == NATIVE_PATH_ENV_VAR {
std::env::set_var(var, val);
std::env::set_var(var, expand_path(val));
break;
}
}

9
crates/nu-engine/src/from_value.rs
View file

@ -3,6 +3,7 @@ use std::path::PathBuf;
use bigdecimal::{BigDecimal, ToPrimitive};
use chrono::{DateTime, FixedOffset};
use nu_errors::ShellError;
use nu_path::expand_path;
use nu_protocol::{
hir::CapturedBlock, ColumnPath, Dictionary, Primitive, Range, SpannedTypeName, UntaggedValue,
Value,
@ -239,11 +240,11 @@ impl FromValue for PathBuf {
Value {
value: UntaggedValue::Primitive(Primitive::String(s)),
..
} => Ok(PathBuf::from(s)),
} => Ok(expand_path(s)),
Value {
value: UntaggedValue::Primitive(Primitive::FilePath(p)),
..
} => Ok(p.clone()),
} => Ok(expand_path(p)),
Value {
value: UntaggedValue::Row(_),
..
@ -265,11 +266,11 @@ impl FromValue for Tagged<PathBuf> {
Value {
value: UntaggedValue::Primitive(Primitive::String(s)),
tag,
} => Ok(PathBuf::from(s).tagged(tag)),
} => Ok(expand_path(s).tagged(tag)),
Value {
value: UntaggedValue::Primitive(Primitive::FilePath(p)),
tag,
} => Ok(p.clone().tagged(tag)),
} => Ok(expand_path(p).tagged(tag)),
Value {
value: UntaggedValue::Row(_),
..

40
crates/nu-parser/src/parse/source.rs
View file

@ -1,11 +1,11 @@
use crate::{lex::tokens::LiteCommand, ParserScope};
use nu_errors::{ArgumentError, ParseError};
use nu_path::expand_path;
use nu_path::{canonicalize, canonicalize_with};
use nu_protocol::hir::{Expression, InternalCommand};
use std::borrow::Cow;
use std::path::Path;
use std::path::PathBuf;
use nu_source::SpannedItem;
pub fn parse_source_internal(
lite_cmd: &LiteCommand,
@ -37,14 +37,14 @@ fn find_source_file(
command: &InternalCommand,
scope: &dyn ParserScope,
) -> Result<(), ParseError> {
let file = if let Some(ref positional_args) = command.args.positional {
let (file, file_span) = if let Some(ref positional_args) = command.args.positional {
if let Expression::FilePath(ref p) = positional_args[0].expr {
p
(p.as_path(), &positional_args[0].span)
} else {
Path::new(&lite_cmd.parts[1].item)
(Path::new(&lite_cmd.parts[1].item), &lite_cmd.parts[1].span)
}
} else {
Path::new(&lite_cmd.parts[1].item)
(Path::new(&lite_cmd.parts[1].item), &lite_cmd.parts[1].span)
};
let lib_dirs = nu_data::config::config(nu_source::Tag::unknown())
@ -61,25 +61,33 @@ fn find_source_file(
if let Some(dir) = lib_dirs {
for lib_path in dir.into_iter().flatten() {
let path = PathBuf::from(lib_path).join(&file);
let path = canonicalize_with(&file, lib_path).map_err(|e| {
ParseError::general_error(
format!("Can't load source file. Reason: {}", e.to_string()),
"Can't load this file".spanned(file_span),
)
})?;
if let Ok(contents) =
std::fs::read_to_string(&expand_path(Cow::Borrowed(path.as_path())))
{
if let Ok(contents) = std::fs::read_to_string(&path) {
return parse(&contents, 0, scope);
}
}
}
let path = Path::new(&file);
let path = canonicalize(&file).map_err(|e| {
ParseError::general_error(
format!("Can't load source file. Reason: {}", e.to_string()),
"Can't load this file".spanned(file_span),
)
})?;
let contents = std::fs::read_to_string(&expand_path(Cow::Borrowed(path)));
let contents = std::fs::read_to_string(&path);
match contents {
Ok(contents) => parse(&contents, 0, scope),
Err(_) => Err(ParseError::argument_error(
lite_cmd.parts[1].clone(),
ArgumentError::BadValue("can't load source file".into()),
Err(e) => Err(ParseError::general_error(
format!("Can't load source file. Reason: {}", e.to_string()),
"Can't load this file".spanned(file_span),
)),
}
}