h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-06-19 13:01:15 -03:00
Code Issues Packages Projects Releases Wiki Activity

Better-named and idiomatic return type for cmdsub parsing

Browse Source 
Commit 2f6ed61833 (parse_util_cmdsubst_extent to return an exclusive
range, 2024-04-27) used the "Parentheses" naming in preparation for
raw quotes.  We still don't have them, so let's clean that up for now.
Also, reuse the Result and Option types.
This commit is contained in:
Johannes Altmanninger
2026-06-17 11:48:54 +08:00
parent 7f9e1a89d6
commit d50c12e142
5 changed files with 96 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
src/expand.rs
View File

@@ -16,7 +16,7 @@
history::{History, history_id},
operation_context::OperationContext,
parse_constants::{ParseError, ParseErrorCode, ParseErrorList, SOURCE_LOCATION_UNKNOWN},
parse_util::{MaybeParentheses, expand_variable_error, locate_cmdsubst_range},
parse_util::{expand_variable_error, locate_cmdsubst_range},
path::path_apply_working_directory,
prelude::*,
wildcard::{WildcardResult, wildcard_expand_string, wildcard_has_internal},
@@ -900,30 +900,30 @@ pub fn expand_cmdsubst(
let mut cursor = 0;
let mut is_quoted = false;
let mut has_dollar = false;
let parens = match locate_cmdsubst_range(
let cmdsub = match locate_cmdsubst_range(
&input,
&mut cursor,
false,
Some(&mut is_quoted),
Some(&mut has_dollar),
) {
MaybeParentheses::Error => {
Err(()) => {
append_syntax_error!(errors, SOURCE_LOCATION_UNKNOWN, "Mismatched parenthesis");
return ExpandResult::make_error(STATUS_EXPAND_ERROR);
}
MaybeParentheses::None => {
Ok(None) => {
if !out.add(input) {
return append_overflow_error(errors, None);
}
return ExpandResult::ok();
}
MaybeParentheses::CommandSubstitution(parens) => parens,
Ok(Some(cmdsub)) => cmdsub,
};
let mut sub_res = vec![];
let job_group = ctx.job_group.clone();
let subshell_status = exec_subshell_for_expand(
&input[parens.command()],
&input[cmdsub.command_range()],
ctx.parser(),
job_group.as_ref(),
&mut sub_res,
@@ -974,12 +974,17 @@ pub fn expand_cmdsubst(
wgettext!("Unknown error while evaluating command substitution")
}
};
append_cmdsub_error_formatted!(errors, parens.start(), parens.end() - 1, err.to_owned());
append_cmdsub_error_formatted!(
errors,
cmdsub.opening_paren_offset(),
cmdsub.end() - 1,
err.to_owned()
);
return ExpandResult::make_error(subshell_status);
}
// Expand slices like (cat /var/words)[1]
let mut tail_begin = parens.end();
let mut tail_begin = cmdsub.end();
if input.as_char_slice().get(tail_begin) == Some(&'[') {
let mut slice_idx = vec![];
let slice_begin = tail_begin;
@@ -1052,9 +1057,15 @@ pub fn expand_cmdsubst(
for tail_item in tail_expand {
let mut whole_item = WString::new();
whole_item.reserve(
parens.start() + 1 + sub_res_joined.len() + 1 + tail_item.completion.len(),
cmdsub.opening_paren_offset()
+ 1
+ sub_res_joined.len()
+ 1
+ tail_item.completion.len(),
);
whole_item.push_utfstr(
&input[..cmdsub.opening_paren_offset() - if has_dollar { 1 } else { 0 }],
);
whole_item.push_utfstr(&input[..parens.start() - if has_dollar { 1 } else { 0 }]);
whole_item.push(INTERNAL_SEPARATOR);
whole_item.push_utfstr(&sub_res_joined);
whole_item.push(INTERNAL_SEPARATOR);
@@ -1071,9 +1082,16 @@ pub fn expand_cmdsubst(
let sub_item2 = escape_string(&sub_item, EscapeStringStyle::Script(EscapeFlags::COMMA));
for tail_item in &*tail_expand {
let mut whole_item = WString::new();
whole_item
.reserve(parens.start() + 1 + sub_item2.len() + 1 + tail_item.completion.len());
whole_item.push_utfstr(&input[..parens.start() - if has_dollar { 1 } else { 0 }]);
whole_item.reserve(
cmdsub.opening_paren_offset()
+ 1
+ sub_item2.len()
+ 1
+ tail_item.completion.len(),
);
whole_item.push_utfstr(
&input[..cmdsub.opening_paren_offset() - if has_dollar { 1 } else { 0 }],
);
whole_item.push(INTERNAL_SEPARATOR);
whole_item.push_utfstr(&sub_item2);
whole_item.push(INTERNAL_SEPARATOR);
@@ -1287,18 +1305,18 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex
if self.flags.contains(ExpandFlags::FAIL_ON_CMDSUBST) {
let mut cursor = 0;
match locate_cmdsubst_range(&input, &mut cursor, true, None, None) {
MaybeParentheses::Error => ExpandResult::make_error(STATUS_EXPAND_ERROR),
MaybeParentheses::None => {
Err(()) => ExpandResult::make_error(STATUS_EXPAND_ERROR),
Ok(None) => {
if !out.add(input) {
return append_overflow_error(self.errors, None);
}
ExpandResult::ok()
}
MaybeParentheses::CommandSubstitution(parens) => {
Ok(Some(cmdsub)) => {
append_cmdsub_error!(
self.errors,
parens.start(),
parens.end() - 1,
cmdsub.opening_paren_offset(),
cmdsub.end() - 1,
"command substitutions not allowed in command position. Try var=(your-cmd) $var ..."
);
ExpandResult::make_error(STATUS_EXPAND_ERROR)

23
src/highlight/highlight.rs
View File

@@ -17,9 +17,7 @@
parse_constants::{
ParseKeyword, ParseTokenType, ParseTreeFlags, SourceRange, StatementDecoration,
},
parse_util::{
MaybeParentheses, get_process_first_token_offset, locate_cmdsubst_range, slice_length,
},
parse_util::{get_process_first_token_offset, locate_cmdsubst_range, slice_length},
path::{path_as_implicit_cd, path_get_cdpath, path_get_path, paths_are_same_file},
terminal::Outputter,
text_face::{ResettableStyle, SpecifiedTextFace, TextFace, UnderlineStyle, parse_text_face},
@@ -839,7 +837,7 @@ fn color_as_argument(&mut self, node: &dyn ast::Node, options_allowed: bool /* =
// Now do command substitutions.
let mut cmdsub_cursor = 0;
let mut is_quoted = false;
while let MaybeParentheses::CommandSubstitution(parens) = locate_cmdsubst_range(
while let Ok(Some(cmdsub)) = locate_cmdsubst_range(
arg_str,
&mut cmdsub_cursor,
/*accept_incomplete=*/ true,
@@ -848,17 +846,17 @@ fn color_as_argument(&mut self, node: &dyn ast::Node, options_allowed: bool /* =
) {
// Highlight the parens. The open parens must exist; the closed paren may not if it was
// incomplete.
assert!(parens.start() < arg_str.len());
self.color_array[arg_start..][parens.opening()]
assert!(cmdsub.opening_paren_offset() < arg_str.len());
self.color_array[arg_start..][cmdsub.opening_paren_range()]
.fill(HighlightSpec::with_fg(HighlightRole::Operat));
self.color_array[arg_start..][parens.closing()]
self.color_array[arg_start..][cmdsub.closing_paren_range()]
.fill(HighlightSpec::with_fg(HighlightRole::Operat));
// Highlight it recursively.
let arg_cursor = self
.cursor
.map(|c| c.wrapping_sub(arg_start + parens.start()));
let cmdsub_contents = &arg_str[parens.command()];
.map(|c| c.wrapping_sub(arg_start + cmdsub.opening_paren_offset()));
let cmdsub_contents = &arg_str[cmdsub.command_range()];
let mut cmdsub_highlighter = Highlighter::new(
cmdsub_contents,
arg_cursor,
@@ -870,7 +868,7 @@ fn color_as_argument(&mut self, node: &dyn ast::Node, options_allowed: bool /* =
// Copy out the subcolors back into our array.
assert_eq!(subcolors.len(), cmdsub_contents.len());
self.color_array[arg_start..][parens.command()].copy_from_slice(&subcolors);
self.color_array[arg_start..][cmdsub.command_range()].copy_from_slice(&subcolors);
}
}
// Colors the source range of a node with a given color.
@@ -1135,9 +1133,8 @@ fn visit_brace_statement(&mut self, brace_statement: &BraceStatement) {
fn has_cmdsub(src: &wstr) -> bool {
let mut cursor = 0;
match locate_cmdsubst_range(src, &mut cursor, true, None, None) {
MaybeParentheses::Error => false,
MaybeParentheses::None => false,
MaybeParentheses::CommandSubstitution(_) => true,
Err(()) | Ok(None) => false,
Ok(Some(_)) => true,
}
}

6
src/parse_execution.rs
View File

@@ -31,9 +31,7 @@
ParseTokenType, StatementDecoration, parse_error_offset_source_start,
},
parse_tree::{NodeRef, ParsedSourceRef},
parse_util::{
MaybeParentheses::CommandSubstitution, locate_cmdsubst_range, unescape_wildcards,
},
parse_util::{locate_cmdsubst_range, unescape_wildcards},
parser::{
Block, BlockData, BlockId, BlockType, LoopStatus, Parser, ParserEnvSetMode, ProfileItem,
},
@@ -1474,7 +1472,7 @@ fn determine_redirections(
None,
None,
) {
CommandSubstitution(p) => p.start() == 0,
Ok(Some(cmdsub)) => cmdsub.opening_paren_offset() == 0,
_ => false,
}
} {

82
src/parse_util.rs
View File

@@ -86,36 +86,29 @@ pub fn slice_length(input: &wstr) -> Option<usize> {
}
#[derive(Debug, Default, Eq, PartialEq)]
pub struct Parentheses {
pub struct CommandSubstitution {
range: Range<usize>,
num_closing: usize,
}
impl Parentheses {
pub fn start(&self) -> usize {
impl CommandSubstitution {
pub fn opening_paren_offset(&self) -> usize {
self.range.start
}
pub fn end(&self) -> usize {
self.range.end
}
pub fn opening(&self) -> Range<usize> {
pub fn opening_paren_range(&self) -> Range<usize> {
self.range.start..self.range.start + 1
}
pub fn closing(&self) -> Range<usize> {
pub fn closing_paren_range(&self) -> Range<usize> {
self.range.end - self.num_closing..self.range.end
}
pub fn command(&self) -> Range<usize> {
pub fn command_range(&self) -> Range<usize> {
self.range.start + 1..self.range.end - self.num_closing
}
}
#[derive(Eq, PartialEq, Debug)]
pub enum MaybeParentheses {
Error,
None,
CommandSubstitution(Parentheses),
}
/// Alternative API. Iterate over command substitutions.
///
/// \param str the string to search for subshells
@@ -130,16 +123,17 @@ pub enum MaybeParentheses {
/// \param out_has_dollar whether the command substitution has the optional leading $.
/// Return -1 on syntax error, 0 if no subshells exist and 1 on success
#[allow(clippy::too_many_arguments)]
#[allow(clippy::result_unit_err)]
pub fn locate_cmdsubst_range(
s: &wstr,
inout_cursor_offset: &mut usize,
accept_incomplete: bool,
inout_is_quoted: Option<&mut bool>,
out_has_dollar: Option<&mut bool>,
) -> MaybeParentheses {
) -> Result<Option<CommandSubstitution>, ()> {
// Nothing to do if the offset is at or past the end of the string.
if *inout_cursor_offset >= s.len() {
return MaybeParentheses::None;
return Ok(None);
}
// Defer to the wonky version.
@@ -151,9 +145,9 @@ pub fn locate_cmdsubst_range(
out_has_dollar,
);
match &ret {
MaybeParentheses::Error | MaybeParentheses::None => (),
MaybeParentheses::CommandSubstitution(parens) => {
*inout_cursor_offset = parens.end();
Err(()) | Ok(None) => (),
Ok(Some(cmdsub)) => {
*inout_cursor_offset = cmdsub.end();
}
}
ret
@@ -173,13 +167,13 @@ pub fn get_cmdsubst_extent(buff: &wstr, cursor: usize) -> ops::Range<usize> {
let mut result = 0..buff.len();
let mut pos = 0;
loop {
let parens = match locate_cmdsub(buff, pos, true, None, None) {
let cmdsub = match locate_cmdsub(buff, pos, true, None, None) {
// No subshell found, all done.
MaybeParentheses::Error | MaybeParentheses::None => break,
MaybeParentheses::CommandSubstitution(parens) => parens,
Err(()) | Ok(None) => break,
Ok(Some(cmdsub)) => cmdsub,
};
let command = parens.command();
let command = cmdsub.command_range();
if command.start <= cursor && command.end >= cursor {
// This command substitution surrounds the cursor, so it's a tighter fit.
result = command;
@@ -196,7 +190,7 @@ pub fn get_cmdsubst_extent(buff: &wstr, cursor: usize) -> ops::Range<usize> {
} else {
// This command substitution ends before the cursor. Skip it.
assert!(command.end < cursor);
pos = parens.end();
pos = cmdsub.end();
assert!(pos <= buff.len());
}
}
@@ -209,7 +203,7 @@ fn locate_cmdsub(
allow_incomplete: bool,
mut inout_is_quoted: Option<&mut bool>,
mut out_has_dollar: Option<&mut bool>,
) -> MaybeParentheses {
) -> Result<Option<CommandSubstitution>, ()> {
let input = input.as_char_slice();
let mut escaped = false;
@@ -352,11 +346,11 @@ fn process_opening_quote(
syntax_error |= paran_count > 0 && !allow_incomplete;
if syntax_error {
return MaybeParentheses::Error;
return Err(());
}
let Some(paran_begin) = paran_begin else {
return MaybeParentheses::None;
return Ok(None);
};
let end = if paran_count != 0 {
@@ -365,12 +359,12 @@ fn process_opening_quote(
paran_end.unwrap() + 1
};
let parens = Parentheses {
let cmdsub = CommandSubstitution {
range: paran_begin..end,
num_closing: if paran_count == 0 { 1 } else { 0 },
};
MaybeParentheses::CommandSubstitution(parens)
Ok(Some(cmdsub))
}
/// Find the beginning and end of the process definition under the cursor
@@ -869,21 +863,21 @@ fn indent_leaf(&mut self, range: SourceRange) {
let mut was_double_quoted;
loop {
was_double_quoted = is_double_quoted;
let parens = match locate_cmdsubst_range(
let cmdsub = match locate_cmdsubst_range(
node_src,
&mut cursor,
/*accept_incomplete=*/ true,
Some(&mut is_double_quoted),
None,
) {
MaybeParentheses::Error => break,
MaybeParentheses::None => {
Err(()) => break,
Ok(None) => {
break;
}
MaybeParentheses::CommandSubstitution(parens) => parens,
Ok(Some(cmdsub)) => cmdsub,
};
let command = parens.command();
let command = cmdsub.command_range();
self.indent_string_part(done..range.start() + command.start, was_double_quoted);
let cmdsub_contents = &node_src[command.clone()];
let indents = compute_indents_from(cmdsub_contents, self.indent + 1);
@@ -891,7 +885,7 @@ fn indent_leaf(&mut self, range: SourceRange) {
.copy_from_slice(&indents);
done = range.start() + command.end;
if parens.closing().is_empty() {
if cmdsub.closing_paren_range().is_empty() {
self.unclosed = true;
}
}
@@ -1402,24 +1396,26 @@ pub fn detect_errors_in_argument(
Some(&mut is_quoted),
Some(&mut has_dollar),
) {
MaybeParentheses::Error => {
Err(()) => {
issue.error = true;
append_syntax_error!(out_errors, source_start, 1, "Mismatched parenthesis");
return Err(issue);
}
MaybeParentheses::None => {
Ok(None) => {
do_loop = false;
}
MaybeParentheses::CommandSubstitution(parens) => {
Ok(Some(cmdsub)) => {
issue.error |= check_subtoken(
checked,
parens.start() - if has_dollar { 1 } else { 0 },
cmdsub.opening_paren_offset() - if has_dollar { 1 } else { 0 },
out_errors,
);
let mut subst_errors = ParseErrorList::new();
if let Err(e) =
detect_parse_errors(&arg_src[parens.command()], Some(&mut subst_errors), false)
{
if let Err(e) = detect_parse_errors(
&arg_src[cmdsub.command_range()],
Some(&mut subst_errors),
false,
) {
issue.error |= e.error;
issue.incomplete |= e.incomplete;
}
@@ -1427,13 +1423,13 @@ pub fn detect_errors_in_argument(
// Our command substitution produced error offsets relative to its source. Tweak the
// offsets of the errors in the command substitution to account for both its offset
// within the string, and the offset of the node.
let error_offset = parens.start() + 1 + source_start;
let error_offset = cmdsub.opening_paren_offset() + 1 + source_start;
parse_error_offset_source_start(&mut subst_errors, error_offset);
if let Some(out_errors) = out_errors {
out_errors.extend(subst_errors);
}
checked = parens.end();
checked = cmdsub.end();
}
}
}

18
src/reader/reader.rs
View File

@@ -68,10 +68,10 @@
panic::AT_EXIT,
parse_constants::{ParseIssue, ParseTreeFlags, SourceRange},
parse_util::{
MaybeParentheses, SPACES_PER_INDENT, compute_indents, contains_wildcards,
detect_parse_errors, escape_wildcards, get_cmdsubst_extent, get_line_from_offset,
get_offset, get_offset_from_line, get_process_extent, get_process_first_token_offset,
get_token_extent, lineno, locate_cmdsubst_range,
SPACES_PER_INDENT, compute_indents, contains_wildcards, detect_parse_errors,
escape_wildcards, get_cmdsubst_extent, get_line_from_offset, get_offset,
get_offset_from_line, get_process_extent, get_process_first_token_offset, get_token_extent,
lineno, locate_cmdsubst_range,
},
parser::{BlockType, EvalRes, Parser, ParserEnvSetMode},
portable_atomic::AtomicU64,
@@ -6084,14 +6084,14 @@ fn extract_tokens(s: &wstr) -> Vec<PositionedToken> {
None,
None,
) {
MaybeParentheses::Error | MaybeParentheses::None => break,
MaybeParentheses::CommandSubstitution(parens) => {
if parens.start() >= range.end() {
Err(()) | Ok(None) => break,
Ok(Some(cmdsub)) => {
if cmdsub.opening_paren_offset() >= range.end() {
break;
}
has_cmd_subs = true;
for mut t in extract_tokens(&s[parens.command()]) {
t.range.start += u32::try_from(parens.command().start).unwrap();
for mut t in extract_tokens(&s[cmdsub.command_range()]) {
t.range.start += u32::try_from(cmdsub.command_range().start).unwrap();
result.push(t);
}
}