rework named args to also contain their short arg (it was there for a reason)

This commit is contained in:
Devyn Cairns 2024-07-09 04:41:36 -07:00
parent 41654e7656
commit f8297abf62
8 changed files with 127 additions and 56 deletions

View File

@ -170,8 +170,12 @@ impl BlockBuilder {
Instruction::StoreEnv { key: _, src } => allocate(&[*src], &[]),
Instruction::PushPositional { src } => allocate(&[*src], &[]),
Instruction::AppendRest { src } => allocate(&[*src], &[]),
Instruction::PushFlag { name: _ } => Ok(()),
Instruction::PushNamed { name: _, src } => allocate(&[*src], &[]),
Instruction::PushFlag { name: _, short: _ } => Ok(()),
Instruction::PushNamed {
name: _,
short: _,
src,
} => allocate(&[*src], &[]),
Instruction::PushParserInfo { name: _, info: _ } => Ok(()),
Instruction::RedirectOut { mode: _ } => Ok(()),
Instruction::RedirectErr { mode: _ } => Ok(()),
@ -310,13 +314,16 @@ impl BlockBuilder {
/// Add data to the `data` array and return a [`DataSlice`] referencing it.
pub(crate) fn data(&mut self, data: impl AsRef<[u8]>) -> Result<DataSlice, CompileError> {
let data = data.as_ref();
let start = self.data.len();
if start + data.as_ref().len() < u32::MAX as usize {
if data.is_empty() {
Ok(DataSlice::empty())
} else if start + data.len() < u32::MAX as usize {
let slice = DataSlice {
start: start as u32,
len: data.as_ref().len() as u32,
len: data.len() as u32,
};
self.data.extend_from_slice(data.as_ref());
self.data.extend_from_slice(data);
Ok(slice)
} else {
Err(CompileError::DataOverflow)

View File

@ -86,7 +86,13 @@ pub(crate) fn compile_call(
// it.
enum CompiledArg<'a> {
Positional(RegId, Span, Option<IrAstRef>),
Named(&'a str, Option<RegId>, Span, Option<IrAstRef>),
Named(
&'a str,
Option<&'a str>,
Option<RegId>,
Span,
Option<IrAstRef>,
),
Spread(RegId, Span, Option<IrAstRef>),
}
@ -125,11 +131,8 @@ pub(crate) fn compile_call(
))
}
Argument::Named((name, short, _)) => compiled_args.push(CompiledArg::Named(
if name.item.is_empty() {
&short.as_ref().expect("no long name and no short name").item
} else {
&name.item
},
&name.item,
short.as_ref().map(|spanned| spanned.item.as_str()),
arg_reg,
arg.span(),
ast_ref,
@ -149,14 +152,23 @@ pub(crate) fn compile_call(
builder.push(Instruction::PushPositional { src: reg }.into_spanned(span))?;
builder.set_last_ast(ast_ref);
}
CompiledArg::Named(name, Some(reg), span, ast_ref) => {
CompiledArg::Named(name, short, Some(reg), span, ast_ref) => {
let name = builder.data(name)?;
builder.push(Instruction::PushNamed { name, src: reg }.into_spanned(span))?;
let short = builder.data(short.unwrap_or(""))?;
builder.push(
Instruction::PushNamed {
name,
short,
src: reg,
}
.into_spanned(span),
)?;
builder.set_last_ast(ast_ref);
}
CompiledArg::Named(name, None, span, ast_ref) => {
CompiledArg::Named(name, short, None, span, ast_ref) => {
let name = builder.data(name)?;
builder.push(Instruction::PushFlag { name }.into_spanned(span))?;
let short = builder.data(short.unwrap_or(""))?;
builder.push(Instruction::PushFlag { name, short }.into_spanned(span))?;
builder.set_last_ast(ast_ref);
}
CompiledArg::Spread(reg, span, ast_ref) => {

View File

@ -378,21 +378,23 @@ fn eval_instruction<D: DebugContext>(
});
Ok(Continue)
}
Instruction::PushFlag { name } => {
Instruction::PushFlag { name, short } => {
let data = ctx.data.clone();
ctx.stack.arguments.push(Argument::Flag {
data,
name: *name,
short: *short,
span: *span,
});
Ok(Continue)
}
Instruction::PushNamed { name, src } => {
Instruction::PushNamed { name, short, src } => {
let val = ctx.collect_reg(*src, *span)?;
let data = ctx.data.clone();
ctx.stack.arguments.push(Argument::Named {
data,
name: *name,
short: *short,
span: *span,
val,
ast: ast.clone().map(|ast_ref| ast_ref.0),
@ -989,21 +991,21 @@ fn eval_call<D: DebugContext>(
result
}
fn find_named_var_id(sig: &Signature, name: &[u8], span: Span) -> Result<VarId, ShellError> {
fn find_named_var_id(
sig: &Signature,
name: &[u8],
short: &[u8],
span: Span,
) -> Result<VarId, ShellError> {
sig.named
.iter()
.find(|n| {
if !n.long.is_empty() {
n.long.as_bytes() == name
} else {
// If the arg has no long name, then compare against its short name
//
// One limitation here is that it effectively makes them share the same namespace,
// if only for args that don't have any long name defined. This probably isn't a
// problem in practice, but TODO it would probably be good to have a parser error
// forbidding a long arg and short arg from having the same name?
// It's possible to only have a short name and no long name
n.short
.is_some_and(|s| s.encode_utf8(&mut [0; 4]).as_bytes() == name)
.is_some_and(|s| s.encode_utf8(&mut [0; 4]).as_bytes() == short)
}
})
.ok_or_else(|| ShellError::IrEvalError {
@ -1087,18 +1089,24 @@ fn gather_arguments(
return Err(ShellError::CannotSpreadAsList { span: vals.span() });
}
}
Argument::Flag { data, name, span } => {
let var_id = find_named_var_id(&block.signature, &data[name], span)?;
Argument::Flag {
data,
name,
short,
span,
} => {
let var_id = find_named_var_id(&block.signature, &data[name], &data[short], span)?;
callee_stack.add_var(var_id, Value::bool(true, span))
}
Argument::Named {
data,
name,
short,
span,
val,
..
} => {
let var_id = find_named_var_id(&block.signature, &data[name], span)?;
let var_id = find_named_var_id(&block.signature, &data[name], &data[short], span)?;
callee_stack.add_var(var_id, val)
}
Argument::ParserInfo { .. } => (),

View File

@ -2,7 +2,7 @@ use nu_engine::command_prelude::*;
use nu_protocol::{
ast::{self, Expr, Expression},
engine::{self, CallImpl, CommandType, UNKNOWN_SPAN_ID},
ir,
ir::{self, DataSlice},
};
#[derive(Clone)]
@ -159,13 +159,15 @@ fn ir_call_to_extern_call(
// Add the arguments, reformatting named arguments into string positionals
for index in 0..call.args_len {
match &call.arguments(stack)[index] {
engine::Argument::Flag { data, name, span } => {
engine::Argument::Flag {
data,
name,
short,
span,
} => {
let name_arg = engine::Argument::Positional {
span: *span,
val: Value::string(
std::str::from_utf8(&data[*name]).expect("invalid flag name"),
*span,
),
val: Value::string(known_external_option_name(&data, *name, *short), *span),
ast: None,
};
extern_call.add_argument(stack, name_arg);
@ -173,16 +175,14 @@ fn ir_call_to_extern_call(
engine::Argument::Named {
data,
name,
short,
span,
val,
..
} => {
let name_arg = engine::Argument::Positional {
span: *span,
val: Value::string(
std::str::from_utf8(&data[*name]).expect("invalid arg name"),
*span,
),
val: Value::string(known_external_option_name(&data, *name, *short), *span),
ast: None,
};
let val_arg = engine::Argument::Positional {
@ -204,3 +204,17 @@ fn ir_call_to_extern_call(
Ok(extern_call.finish())
}
fn known_external_option_name(data: &[u8], name: DataSlice, short: DataSlice) -> String {
if !data[name].is_empty() {
format!(
"--{}",
std::str::from_utf8(&data[name]).expect("invalid utf-8 in flag name")
)
} else {
format!(
"-{}",
std::str::from_utf8(&data[short]).expect("invalid utf-8 in flag short name")
)
}
}

View File

@ -21,12 +21,14 @@ pub enum Argument {
Flag {
data: Arc<[u8]>,
name: DataSlice,
short: DataSlice,
span: Span,
},
/// A named argument with a value, e.g. `--flag value` or `--flag=`
Named {
data: Arc<[u8]>,
name: DataSlice,
short: DataSlice,
span: Span,
val: Value,
ast: Option<Arc<Expression>>,

View File

@ -270,13 +270,23 @@ impl CallBuilder {
}
/// Add a flag (no-value named) argument to the [`Stack`] and reference it from the [`Call`].
pub fn add_flag(&mut self, stack: &mut Stack, name: impl AsRef<str>, span: Span) -> &mut Self {
let data: Arc<[u8]> = name.as_ref().as_bytes().into();
let name = DataSlice {
start: 0,
len: data.len().try_into().expect("flag name too big"),
};
self.add_argument(stack, Argument::Flag { data, name, span })
pub fn add_flag(
&mut self,
stack: &mut Stack,
name: impl AsRef<str>,
short: impl AsRef<str>,
span: Span,
) -> &mut Self {
let (data, name, short) = data_from_name_and_short(name.as_ref(), short.as_ref());
self.add_argument(
stack,
Argument::Flag {
data,
name,
short,
span,
},
)
}
/// Add a named argument to the [`Stack`] and reference it from the [`Call`].
@ -284,19 +294,17 @@ impl CallBuilder {
&mut self,
stack: &mut Stack,
name: impl AsRef<str>,
short: impl AsRef<str>,
span: Span,
val: Value,
) -> &mut Self {
let data: Arc<[u8]> = name.as_ref().as_bytes().into();
let name = DataSlice {
start: 0,
len: data.len().try_into().expect("arg name too big"),
};
let (data, name, short) = data_from_name_and_short(name.as_ref(), short.as_ref());
self.add_argument(
stack,
Argument::Named {
data,
name,
short,
span,
val,
ast: None,
@ -327,3 +335,17 @@ impl CallBuilder {
result
}
}
fn data_from_name_and_short(name: &str, short: &str) -> (Arc<[u8]>, DataSlice, DataSlice) {
let data: Vec<u8> = name.bytes().chain(short.bytes()).collect();
let data: Arc<[u8]> = data.into();
let name = DataSlice {
start: 0,
len: name.len().try_into().expect("flag name too big"),
};
let short = DataSlice {
start: name.start.checked_add(name.len).expect("flag name too big"),
len: short.len().try_into().expect("flag short name too big"),
};
(data, name, short)
}

View File

@ -112,13 +112,15 @@ impl<'a> fmt::Display for FmtInstruction<'a> {
Instruction::AppendRest { src } => {
write!(f, "{:WIDTH$} {src}", "append-rest")
}
Instruction::PushFlag { name } => {
Instruction::PushFlag { name, short } => {
let name = FmtData(self.data, *name);
write!(f, "{:WIDTH$} {name}", "push-flag")
let short = FmtData(self.data, *short);
write!(f, "{:WIDTH$} {name}, {short}", "push-flag")
}
Instruction::PushNamed { name, src } => {
Instruction::PushNamed { name, short, src } => {
let name = FmtData(self.data, *name);
write!(f, "{:WIDTH$} {name}, {src}", "push-named")
let short = FmtData(self.data, *short);
write!(f, "{:WIDTH$} {name}, {short}, {src}", "push-named")
}
Instruction::PushParserInfo { name, info } => {
let name = FmtData(self.data, *name);

View File

@ -134,9 +134,13 @@ pub enum Instruction {
/// Add a list of args to the next (internal) call (spread/rest).
AppendRest { src: RegId },
/// Add a named arg with no value to the next (internal) call.
PushFlag { name: DataSlice },
PushFlag { name: DataSlice, short: DataSlice },
/// Add a named arg with a value to the next (internal) call.
PushNamed { name: DataSlice, src: RegId },
PushNamed {
name: DataSlice,
short: DataSlice,
src: RegId,
},
/// Add parser info to the next (internal) call.
PushParserInfo {
name: DataSlice,