h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-05-24 05:41:14 -03:00
Code Issues Packages Projects Releases Wiki Activity

Back out "Allow if/then/fi, while/do/done and for/do/done"

Browse Source 
This backs out commit e32572e4e6.
This commit is contained in:
Johannes Altmanninger
2025-01-19 18:49:05 +01:00
parent 5b0622cf00
commit 92bd366c1b
9 changed files with 11 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
share/completions/do.fish
View File

@@ -1 +0,0 @@
complete -c do -xa '(__fish_complete_subcommand)'

1
share/completions/then.fish
View File

@@ -1 +0,0 @@
complete -c then -xa '(__fish_complete_subcommand)'

99
src/ast.rs
View File

@@ -79,10 +79,8 @@ fn visit_decorated_statement_decorator(
_node: &mut Option<DecoratedStatementDecorator>,
);
fn visit_job_conjunction_decorator(&mut self, _node: &mut Option<JobConjunctionDecorator>);
fn visit_do(&mut self, _node: &mut Option<KeywordDo>);
fn visit_else_clause(&mut self, _node: &mut Option<ElseClause>);
fn visit_semi_nl(&mut self, _node: &mut Option<SemiNl>);
fn visit_then(&mut self, _node: &mut Option<KeywordThen>);
fn visit_time(&mut self, _node: &mut Option<KeywordTime>);
fn visit_token_background(&mut self, _node: &mut Option<TokenBackground>);
}
@@ -979,12 +977,6 @@ macro_rules! visit_optional_field_mut {
(KeywordTime, $field:expr, $visitor:ident) => {
$visitor.visit_time(&mut $field);
};
(KeywordDo, $field:expr, $visitor:ident) => {
$visitor.visit_do(&mut $field);
};
(KeywordThen, $field:expr, $visitor:ident) => {
$visitor.visit_then(&mut $field);
};
(TokenBackground, $field:expr, $visitor:ident) => {
$visitor.visit_token_background(&mut $field);
};
@@ -1323,11 +1315,7 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool {
|| (token.typ == ParseTokenType::string
&& !matches!(
token.keyword,
ParseKeyword::kw_case
| ParseKeyword::kw_done
| ParseKeyword::kw_else
| ParseKeyword::kw_end
| ParseKeyword::kw_fi
ParseKeyword::kw_case | ParseKeyword::kw_end | ParseKeyword::kw_else
))
}
}
@@ -1345,8 +1333,6 @@ pub struct ForHeader {
pub args: ArgumentList,
/// newline or semicolon
pub semi_nl: SemiNl,
// Optional "do" for compatibility with POSIX.
pub kw_do: Option<KeywordDo>,
}
implement_node!(ForHeader, branch, for_header);
implement_acceptor_for_branch!(
@@ -1356,7 +1342,6 @@ pub struct ForHeader {
(kw_in: (KeywordIn)),
(args: (ArgumentList)),
(semi_nl: (SemiNl)),
(kw_do: (Option<KeywordDo>)),
);
impl ConcreteNode for ForHeader {
fn as_for_header(&self) -> Option<&ForHeader> {
@@ -1376,8 +1361,6 @@ pub struct WhileHeader {
pub kw_while: KeywordWhile,
pub condition: JobConjunction,
pub andor_tail: AndorJobList,
// Optional "do" for compatibility with POSIX.
pub kw_do: Option<KeywordDo>,
}
implement_node!(WhileHeader, branch, while_header);
implement_acceptor_for_branch!(
@@ -1385,7 +1368,6 @@ pub struct WhileHeader {
(kw_while: (KeywordWhile)),
(condition: (JobConjunction)),
(andor_tail: (AndorJobList)),
(kw_do: (Option<KeywordDo>)),
);
impl ConcreteNode for WhileHeader {
fn as_while_header(&self) -> Option<&WhileHeader> {
@@ -1459,7 +1441,7 @@ pub struct BlockStatement {
/// List of jobs in this block.
pub jobs: JobList,
/// The 'end' node.
pub end: KeywordEndOrDone,
pub end: KeywordEnd,
/// Arguments and redirections associated with the block.
pub args_or_redirs: ArgumentOrRedirectionList,
}
@@ -1468,7 +1450,7 @@ pub struct BlockStatement {
BlockStatement,
(header: (variant<BlockStatementHeaderVariant>)),
(jobs: (JobList)),
(end: (KeywordEndOrDone)),
(end: (KeywordEnd)),
(args_or_redirs: (ArgumentOrRedirectionList)),
);
impl ConcreteNode for BlockStatement {
@@ -1522,10 +1504,6 @@ pub struct IfClause {
pub condition: JobConjunction,
/// 'and/or' tail.
pub andor_tail: AndorJobList,
// Semi
pub semi_nl: SemiNl,
// Optional "then" for compatibility with POSIX.
pub kw_then: Option<KeywordThen>,
/// The body to execute if the condition is true.
pub body: JobList,
}
@@ -1535,7 +1513,6 @@ pub struct IfClause {
(kw_if: (KeywordIf)),
(condition: (JobConjunction)),
(andor_tail: (AndorJobList)),
(kw_then: (Option<KeywordThen>)),
(body: (JobList)),
);
impl ConcreteNode for IfClause {
@@ -1633,7 +1610,7 @@ pub struct IfStatement {
/// else part
pub else_clause: Option<ElseClause>,
/// literal end
pub end: KeywordEndOrFi,
pub end: KeywordEnd,
/// block args / redirs
pub args_or_redirs: ArgumentOrRedirectionList,
}
@@ -1643,7 +1620,7 @@ pub struct IfStatement {
(if_clause: (IfClause)),
(elseif_clauses: (ElseifClauseList)),
(else_clause: (Option<ElseClause>)),
(end: (KeywordEndOrFi)),
(end: (KeywordEnd)),
(args_or_redirs: (ArgumentOrRedirectionList)),
);
impl ConcreteNode for IfStatement {
@@ -2096,18 +2073,14 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool {
define_keyword_node!(JobConjunctionDecorator, kw_and, kw_or);
define_keyword_node!(KeywordBegin, kw_begin);
define_keyword_node!(KeywordCase, kw_case);
define_keyword_node!(KeywordDo, kw_do);
define_keyword_node!(KeywordElse, kw_else);
define_keyword_node!(KeywordEnd, kw_end);
define_keyword_node!(KeywordEndOrDone, kw_end, kw_done);
define_keyword_node!(KeywordEndOrFi, kw_end, kw_fi);
define_keyword_node!(KeywordFor, kw_for);
define_keyword_node!(KeywordFunction, kw_function);
define_keyword_node!(KeywordIf, kw_if);
define_keyword_node!(KeywordIn, kw_in);
define_keyword_node!(KeywordNot, kw_not, kw_exclam);
define_keyword_node!(KeywordSwitch, kw_switch);
define_keyword_node!(KeywordThen, kw_then);
define_keyword_node!(KeywordTime, kw_time);
define_keyword_node!(KeywordWhile, kw_while);
@@ -2152,26 +2125,6 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool {
}
}
impl CheckParse for KeywordDo {
fn can_be_parsed(pop: &mut Populator<'_>) -> bool {
let keyword = pop.peek_token(0).keyword;
if !matches!(keyword, ParseKeyword::kw_do) {
return false;
}
!pop.peek_token(1).is_dash_prefix_string()
}
}
impl CheckParse for KeywordThen {
fn can_be_parsed(pop: &mut Populator<'_>) -> bool {
let keyword = pop.peek_token(0).keyword;
if !matches!(keyword, ParseKeyword::kw_then) {
return false;
}
!pop.peek_token(1).is_dash_prefix_string()
}
}
impl DecoratedStatement {
/// Return the decoration for this statement.
pub fn decoration(&self) -> StatementDecoration {
@@ -3208,10 +3161,7 @@ fn did_visit_fields_of<'a>(&'a mut self, node: &'a dyn NodeMut, flow: VisitResul
if next_token.typ == ParseTokenType::string
&& matches!(
next_token.keyword,
ParseKeyword::kw_case
| ParseKeyword::kw_else
| ParseKeyword::kw_end
| ParseKeyword::kw_fi
ParseKeyword::kw_case | ParseKeyword::kw_else | ParseKeyword::kw_end
)
{
self.consume_excess_token_generating_error();
@@ -3295,12 +3245,6 @@ fn visit_semi_nl(&mut self, node: &mut Option<SemiNl>) {
fn visit_time(&mut self, node: &mut Option<KeywordTime>) {
*node = self.try_parse::<KeywordTime>().map(|b| *b);
}
fn visit_do(&mut self, node: &mut Option<KeywordDo>) {
*node = self.try_parse::<KeywordDo>().map(|b| *b);
}
fn visit_then(&mut self, node: &mut Option<KeywordThen>) {
*node = self.try_parse::<KeywordThen>().map(|b| *b);
}
fn visit_token_background(&mut self, node: &mut Option<TokenBackground>) {
*node = self.try_parse::<TokenBackground>().map(|b| *b);
}
@@ -3602,14 +3546,6 @@ fn consume_excess_token_generating_error(&mut self) {
"'case' builtin not inside of switch block"
);
}
ParseKeyword::kw_done => {
parse_error!(
self,
tok,
ParseErrorCode::unbalancing_case,
"'done' outside of a block"
);
}
ParseKeyword::kw_end => {
parse_error!(
self,
@@ -3626,14 +3562,6 @@ fn consume_excess_token_generating_error(&mut self) {
"'else' builtin not inside of if block"
);
}
ParseKeyword::kw_fi => {
parse_error!(
self,
tok,
ParseErrorCode::unbalancing_end,
"'fi' outside of a block"
);
}
_ => {
internal_error!(
self,
@@ -3891,11 +3819,9 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant {
// looks like an option (starts with a dash), then parse it as a decorated statement.
let help_only_kws = [
ParseKeyword::kw_begin,
ParseKeyword::kw_do,
ParseKeyword::kw_function,
ParseKeyword::kw_if,
ParseKeyword::kw_switch,
ParseKeyword::kw_then,
ParseKeyword::kw_while,
];
if if help_only_kws.contains(&self.peek_token(0).keyword) {
@@ -3908,13 +3834,8 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant {
// Likewise if the next token doesn't look like an argument at all. This corresponds to
// e.g. a "naked if".
let naked_invocation_invokes_help = ![
ParseKeyword::kw_begin,
ParseKeyword::kw_done,
ParseKeyword::kw_end,
ParseKeyword::kw_fi,
]
.contains(&self.peek_token(0).keyword);
let naked_invocation_invokes_help = ![ParseKeyword::kw_begin, ParseKeyword::kw_end]
.contains(&self.peek_token(0).keyword);
if naked_invocation_invokes_help
&& [ParseTokenType::end, ParseTokenType::terminate]
.contains(&self.peek_token(1).typ)
@@ -3943,7 +3864,7 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant {
let embedded = self.allocate_visit::<SwitchStatement>();
StatementVariant::SwitchStatement(embedded)
}
ParseKeyword::kw_done | ParseKeyword::kw_end | ParseKeyword::kw_fi => {
ParseKeyword::kw_end => {
// 'end' is forbidden as a command.
// For example, `if end` or `while end` will produce this error.
// We still have to descend into the decorated statement because
@@ -4115,7 +4036,7 @@ fn visit_keyword(&mut self, keyword: &mut dyn Keyword) -> VisitResult {
// Special error reporting for keyword_t<kw_end>.
let allowed_keywords = keyword.allowed_keywords();
if allowed_keywords.contains(&ParseKeyword::kw_end) {
if keyword.allowed_keywords() == [ParseKeyword::kw_end] {
return VisitResult::Break(MissingEndError {
allowed_keywords,
token: *self.peek_token(0),

20
src/builtins/shared.rs
View File

@@ -209,14 +209,6 @@ struct BuiltinData {
name: L!("disown"),
func: disown::disown,
},
BuiltinData {
name: L!("do"),
func: builtin_generic,
},
BuiltinData {
name: L!("done"),
func: builtin_generic,
},
BuiltinData {
name: L!("echo"),
func: echo::echo,
@@ -253,10 +245,6 @@ struct BuiltinData {
name: L!("fg"),
func: fg::fg,
},
BuiltinData {
name: L!("fi"),
func: builtin_generic,
},
BuiltinData {
name: L!("fish_indent"),
func: fish_indent::fish_indent,
@@ -357,10 +345,6 @@ struct BuiltinData {
name: L!("test"),
func: test::test,
},
BuiltinData {
name: L!("then"),
func: builtin_generic,
},
BuiltinData {
name: L!("time"),
func: builtin_generic,
@@ -520,8 +504,6 @@ pub fn builtin_get_desc(name: &wstr) -> Option<&'static wstr> {
_ if name == "continue" => wgettext!("Skip over remaining innermost loop"),
_ if name == "count" => wgettext!("Count the number of arguments"),
_ if name == "disown" => wgettext!("Remove job from job list"),
_ if name == "do" => wgettext!("Optional after a loop header"),
_ if name == "done" => wgettext!("Alternative to 'end', to end a block of commands"),
_ if name == "echo" => wgettext!("Print arguments"),
_ if name == "else" => wgettext!("Evaluate block if condition is false"),
_ if name == "emit" => wgettext!("Emit an event"),
@@ -531,7 +513,6 @@ pub fn builtin_get_desc(name: &wstr) -> Option<&'static wstr> {
_ if name == "exit" => wgettext!("Exit the shell"),
_ if name == "false" => wgettext!("Return an unsuccessful result"),
_ if name == "fg" => wgettext!("Send job to foreground"),
_ if name == "fi" => wgettext!("Alternative to 'end', to end a block of commands"),
_ if name == "fish_key_reader" => wgettext!("explore what characters keyboard keys send"),
_ if name == "for" => wgettext!("Perform a set of commands multiple times"),
_ if name == "function" => wgettext!("Define a new function"),
@@ -550,7 +531,6 @@ pub fn builtin_get_desc(name: &wstr) -> Option<&'static wstr> {
_ if name == "realpath" => wgettext!("Show absolute path sans symlinks"),
_ if name == "return" => wgettext!("Stop the currently evaluated function"),
_ if name == "set" => wgettext!("Handle environment variables"),
_ if name == "then" => wgettext!("Optional after the condition of an if-statement"),
_ if name == "set_color" => wgettext!("Set the terminal color"),
_ if name == "source" => wgettext!("Evaluate contents of file"),
_ if name == "status" => wgettext!("Return status information about fish"),

4
src/highlight/highlight.rs
View File

@@ -846,18 +846,14 @@ fn visit_keyword(&mut self, node: &dyn Keyword) {
| ParseKeyword::kw_builtin
| ParseKeyword::kw_case
| ParseKeyword::kw_command
| ParseKeyword::kw_do
| ParseKeyword::kw_done
| ParseKeyword::kw_else
| ParseKeyword::kw_end
| ParseKeyword::kw_exec
| ParseKeyword::kw_fi
| ParseKeyword::kw_for
| ParseKeyword::kw_function
| ParseKeyword::kw_if
| ParseKeyword::kw_in
| ParseKeyword::kw_switch
| ParseKeyword::kw_then
| ParseKeyword::kw_while => role = HighlightRole::keyword,
ParseKeyword::kw_and
| ParseKeyword::kw_or

12
src/parse_constants.rs
View File

@@ -92,13 +92,10 @@ pub enum ParseKeyword {
kw_builtin,
kw_case,
kw_command,
kw_do,
kw_done,
kw_else,
kw_end,
kw_exclam,
kw_exec,
kw_fi,
kw_for,
kw_function,
kw_if,
@@ -106,7 +103,6 @@ pub enum ParseKeyword {
kw_not,
kw_or,
kw_switch,
kw_then,
kw_time,
kw_while,
}
@@ -241,13 +237,10 @@ pub fn to_wstr(self) -> &'static wstr {
ParseKeyword::kw_builtin => L!("builtin"),
ParseKeyword::kw_case => L!("case"),
ParseKeyword::kw_command => L!("command"),
ParseKeyword::kw_do => L!("do"),
ParseKeyword::kw_done => L!("done"),
ParseKeyword::kw_else => L!("else"),
ParseKeyword::kw_end => L!("end"),
ParseKeyword::kw_exclam => L!("!"),
ParseKeyword::kw_exec => L!("exec"),
ParseKeyword::kw_fi => L!("fi"),
ParseKeyword::kw_for => L!("for"),
ParseKeyword::kw_function => L!("function"),
ParseKeyword::kw_if => L!("if"),
@@ -255,7 +248,6 @@ pub fn to_wstr(self) -> &'static wstr {
ParseKeyword::kw_not => L!("not"),
ParseKeyword::kw_or => L!("or"),
ParseKeyword::kw_switch => L!("switch"),
ParseKeyword::kw_then => L!("then"),
ParseKeyword::kw_time => L!("time"),
ParseKeyword::kw_while => L!("while"),
_ => L!("unknown_keyword"),
@@ -278,13 +270,10 @@ fn from(s: &wstr) -> Self {
_ if s == "builtin" => ParseKeyword::kw_builtin,
_ if s == "case" => ParseKeyword::kw_case,
_ if s == "command" => ParseKeyword::kw_command,
_ if s == "do" => ParseKeyword::kw_do,
_ if s == "done" => ParseKeyword::kw_done,
_ if s == "else" => ParseKeyword::kw_else,
_ if s == "end" => ParseKeyword::kw_end,
_ if s == "exec" => ParseKeyword::kw_exec,
_ if s == "for" => ParseKeyword::kw_for,
_ if s == "fi" => ParseKeyword::kw_fi,
_ if s == "function" => ParseKeyword::kw_function,
_ if s == "if" => ParseKeyword::kw_if,
_ if s == "in" => ParseKeyword::kw_in,
@@ -292,7 +281,6 @@ fn from(s: &wstr) -> Self {
_ if s == "or" => ParseKeyword::kw_or,
_ if s == "switch" => ParseKeyword::kw_switch,
_ if s == "time" => ParseKeyword::kw_time,
_ if s == "then" => ParseKeyword::kw_then,
_ if s == "while" => ParseKeyword::kw_while,
_ => ParseKeyword::none,
}

2
src/parse_tree.rs
View File

@@ -15,7 +15,7 @@
use crate::wcstringutil::count_newlines;
/// A struct representing the token type that we use internally.
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy)]
pub struct ParseToken {
/// The type of the token as represented by the parser
pub typ: ParseTokenType,

4
src/parser_keywords.rs
View File

@@ -50,13 +50,10 @@ macro_rules! reserved_words {
("case"),
("command", [subcommand]),
("continue"),
("do", [subcommand]),
("done"),
("else", [subcommand]),
("end"),
("eval"),
("exec", [subcommand]),
("fi"),
("for"),
("function"),
("if", [subcommand]),
@@ -68,7 +65,6 @@ macro_rules! reserved_words {
("status"),
("string"),
("switch"),
("then"),
("test"),
("time", [subcommand]),
("while", [subcommand]),

9
tests/checks/indent.fish
View File

@@ -614,12 +614,3 @@ echo 'echo "foo" "bar"' > $tmpdir/indent_test.fish
$fish_indent --write $tmpdir/indent_test.fish
cat $tmpdir/indent_test.fish
# CHECK: echo foo bar
# TODO: We should not force the line break.
echo 'if true; then
echo body
fi' | $fish_indent
# CHECK: if true
# CHECK: then
# CHECK: {{ }}echo body
# CHECK: fi