From 91e1d598695d9b4ed6e1b5b32e035155e611bc89 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Wed, 17 Oct 2012 01:07:34 -0700 Subject: [PATCH] Fix for issue where else if would fail to pass arguments to commands. Also implements short-circuiting for and/or so that non-existent commands don't produce error messages. Fixes https://github.com/fish-shell/fish-shell/issues/345 Fixes https://github.com/fish-shell/fish-shell/issues/349 --- parser.cpp | 31 ++++++++++++++++++++++++++++--- tests/test7.in | 9 +++++++++ tests/test7.out | 1 + 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/parser.cpp b/parser.cpp index ab79bc4ba..72cd4be10 100644 --- a/parser.cpp +++ b/parser.cpp @@ -365,6 +365,7 @@ static const struct block_lookup_entry block_lookup[]= } }; +static bool job_should_skip_elseif(const job_t *job, const block_t *current_block); parser_t::parser_t(enum parser_type_t type, bool errors) : parser_type(type), @@ -1413,7 +1414,19 @@ void parser_t::parse_job_argument_list( process_t *p, { skip=0; } + else if (job_get_flag(j, JOB_ELSEIF) && ! job_should_skip_elseif(j, current_block)) + { + skip=0; + } } + else + { + /* If this is an else if, and we should skip it, then don't expand any arguments */ + if (job_get_flag(j, JOB_ELSEIF) && job_should_skip_elseif(j, current_block)) + { + skip = 1; + } + } if( !skip ) { @@ -1705,6 +1718,7 @@ int parser_t::parse_job( process_t *p, int use_command = 1; // May commands be considered when checking what action this command represents int is_new_block=0; // Does this command create a new block? bool unskip = false; // Maybe we are an elseif inside an if block; if so we may want to evaluate this even if the if block is currently set to skip + bool allow_bogus_command = false; // If we are an elseif that will not be executed, or an AND or OR that will have been short circuited, don't complain about non-existent commands block_t *prev_block = current_block; int prev_tokenizer_pos = current_tokenizer_pos; @@ -1841,11 +1855,15 @@ int parser_t::parse_job( process_t *p, } else if( nxt == L"and" ) { - job_set_flag( j, JOB_SKIP, proc_get_last_status()); + bool skip = (proc_get_last_status() != 0); + job_set_flag( j, JOB_SKIP, skip); + allow_bogus_command = skip; } else if( nxt == L"or" ) { - job_set_flag( j, JOB_SKIP, !proc_get_last_status()); + bool skip = (proc_get_last_status() == 0); + job_set_flag( j, JOB_SKIP, skip); + allow_bogus_command = skip; } else if( is_exec ) { @@ -1928,6 +1946,10 @@ int parser_t::parse_job( process_t *p, /* We want to execute this ELSEIF if the IF expression was evaluated, it failed, and so has every other ELSEIF (if any) */ unskip = (ib->if_expr_evaluated && ! ib->any_branch_taken); + + /* But if we're not executing it, don't complain about its command if it doesn't exist */ + if (! unskip) + allow_bogus_command = true; } } } @@ -2015,7 +2037,10 @@ int parser_t::parse_job( process_t *p, If we are not executing the current block, allow non-existent commands. */ - if( current_block->skip && ! unskip) + if (current_block->skip && ! unskip) + allow_bogus_command = true; //note this may already be true for other reasons + + if (allow_bogus_command) { p->actual_cmd.clear(); } diff --git a/tests/test7.in b/tests/test7.in index 8e80da59e..910f80805 100644 --- a/tests/test7.in +++ b/tests/test7.in @@ -93,3 +93,12 @@ else echo delta4.1 echo delta4.2 end + +if test ! -n "abc" +else if test -n "def" + echo "epsilon5.2" +else if not_a_valid_command but it should be OK because a previous branch was taken + echo "epsilon 5.3" +else if test ! -n "abc" + echo "epsilon 5.4" +end diff --git a/tests/test7.out b/tests/test7.out index 454240b4a..903f1973e 100644 --- a/tests/test7.out +++ b/tests/test7.out @@ -23,3 +23,4 @@ yep4.1 yep4.2 delta4.1 delta4.2 +epsilon5.2